Foundation Classes - Optimise AsciiString with pre-defined string - #752
Merged
Merged
Conversation
…w API - Rename internal members mystring/mylength -> myString/myLength across AsciiString implementation and related files to improve naming consistency. - Add std::string_view support and efficient overloads: - constructor from string_view, AssignCat/Copy/Insert/SetValue from string_view - Cat, operators (+, +=) and comparison helpers accepting string_view - Token/StartsWith/EndsWith/Search helpers and literal-template overloads - operator std::string_view() to obtain a non-owning view - IsEqual/IsSameString/IsDifferent variants for string_view/C-string combinations - optimized literal templates to avoid runtime strlen for compile-time literals - Improve memory handling and allocation helpers (allocate/reallocate/deallocate) and keep padding strategy consistent. - Add Insert/AssignCat implementations that accept string_view and use memmove/memcpy. - Make TCollection_ExtendedString C-string constructor explicit. - Fix calls in HAsciiString to use renamed members. - Adjust misc. parameter names and small code-style/clarity improvements. These changes enable zero-copy interop with std::string_view, reduce redundant C-string operations for literals, and unify internal naming for maintainability.
… API
- Rework TCollection_AsciiString to use core pointer+length implementations for
Copy, AssignCat, Search, SearchFromEnd, StartsWith, EndsWith,
FirstLocationInSet/FirstLocationNotInSet and others.
- Provide overloads and convenience wrappers for std::string_view, C-strings,
string literals and TCollection_AsciiString that delegate to the central
implementations.
- Inline numerous trivial methods in the header (Length, ToCString, Cat,
comparison helpers) and add std::hash<TCollection_AsciiString> specialization.
- Remove the deprecated TCollection_AsciiString.lxx file and update FILES.cmake.
- Ensure correct null-termination and safer handling of null/empty inputs in
AssignCat/Copy paths.
- Adapt Message-related code to the new string APIs:
- Message_Msg: convert TCollection_ExtendedString key to TCollection_AsciiString
before looking up message file entries.
- Message_MsgFile: use TCollection_ExtendedString for constructed error message.
- Message_Algorithm: build reports using TCollection_ExtendedString wrappers
instead of implicit ASCIIC string concatenation.
These changes modernize string handling, enable efficient std::string_view usage
and consolidate core behaviors while keeping backward-compatible overloads.
…elax explicit ctors - Move TCollection_AsciiString::SubString implementation from header to .cxx. - Add bounds check (raises Standard_OutOfRange) and use casts to suppress -Wstrict-overflow. - Mark SubString (and AsciiString ctor from ExtendedString) with Standard_EXPORT. - Remove explicit from a few string constructors (char-literal ctor and ExtendedString/Ascii cstring ctor) to align with other CTORs. This ensures proper symbol export, reduces header inlining, and fixes compiler warnings.
dpasukhi
added
2. Enhancement
Replace TCollection_AsciiString.lxx with TCollection_HAsciiString.lxx in the package file list so the HAsciiString implementation file is properly referenced.
…ternals - Add core overloads that accept (const Standard_CString, length) for Insert, SetValue, IsEqual, IsLess, IsGreater, IsDifferent and IsSameString; existing string_view/C-string/AsciiString overloads are redirected to these. - Improve AssignCat(int/double) to format into a stack buffer (Sprintf) and append without creating temporary AsciiString. - Fix self-reference / overlap handling in AssignCat(const C*,len) and Insert(...) by copying source data before/after reallocation and using memmove where needed. - Simplify comparisons and searches using memcmp/memmove for performance and clear 1-based position return semantics in Search/SearchFromEnd. - Add missing Standard_OutOfRange includes where required. - Fix HAsciiString accessors (use ToCString()/Length()) and lxx Length() helper. These changes modernize internal APIs, reduce allocations/copies and make behavior for overlapping inserts/concats correct and efficient.
… internals - Add Cat(const Standard_CString, Standard_Integer) as the core pointer+length append API; make other Cat overloads delegate to it (char, C-string, string_view, literal, TCollection_AsciiString). - Introduce FormattedInteger and FormattedReal helpers to centralize integer/real formatting and remove duplicated stack buffers; use them from constructors and AssignCat/Cat. - Optimize Copy(): handle self-pointer, reuse existing buffer when capacity suffices, avoid unnecessary deallocation and ensure null terminator. - Move small Cat implementations inline in header and remove out-of-line std::string_view duplicate to simplify codegen and reduce redundancy.
…Cat/Insert/Cat/Copy/Search/comparison/SetValue/Remove/Trunc/string_view/move/template-literal and stress/edge cases
- Wrap inclusion of <string_view> and all string_view-based ctors/operators/methods with C++17 checks to preserve pre-C++17 compatibility. - Inline explicit operator std::string_view in the header and remove its out-of-line definition. - Replace temporary std::string_view usages in StartsWith/EndsWith/IsSameString with pointer+length overloads to use core APIs consistently.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the TCollection_AsciiString class to optimize performance with pre-defined strings by replacing the .lxx inline implementation file with modern C++ optimizations. The changes implement compile-time size deduction for string literals, unify string operations with core implementations, and add comprehensive C++17 string_view support.
Key changes include:
- Elimination of the .lxx file in favor of inline methods within the header
- Template constructors and methods for compile-time string literal optimization
- Core implementation pattern where all overloads redirect to primary implementations
- Enhanced C++17 string_view integration throughout the API
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TCollection_AsciiString.lxx | Complete removal of the inline implementation file |
| TCollection_AsciiString.hxx | Major refactor with inlined methods, template optimizations, and comprehensive API redesign |
| TCollection_AsciiString.cxx | Updated implementation with core methods, self-reference safety, and optimized string operations |
| TCollection_HAsciiString.lxx | Updated to use public Length() method instead of direct member access |
| TCollection_HAsciiString.cxx | Updated to use ToCString() method and added missing include |
| TCollection_ExtendedString.cxx | Added missing Standard_OutOfRange include |
| FILES.cmake | Removed reference to deleted .lxx file |
| TCollection_AsciiString_Test.cxx | Added comprehensive test suite covering new functionality |
Comments suppressed due to low confidence (1)
src/FoundationClasses/TKernel/TCollection/TCollection_AsciiString.cxx:1
- The
calculatePaddedSizefunction has a different implementation than the context version shown. The context shows multiplication bysizeof(Standard_ExtCharacter)which is missing here, potentially causing incorrect memory allocation calculations.
// Copyright (c) 1993-1999 Matra Datavision
…ll-termination, and expose pointer+length Insert API - Tests: add comprehensive Insert overlap unit tests and a regression test for multiple AssignCat in loop. - Fix TCollection_AsciiString::AssignCat to always null-terminate when extending the buffer. - Fix self-referential Insert: compute shifted source offset after reallocation/shift and use it with memmove to correctly handle overlapping regions. - Ensure reallocate() preserves null-termination consistently. - API: introduce core pointer+length overloads for InsertAfter/InsertBefore and add forwarding overloads for: - TCollection_AsciiString, C-string, string_view (C++17) and template char-array forms. - Template overloads and other literal helpers now use strlen() to safely handle both true string literals and char arrays/buffers. - Small doc/comment updates to reflect behavior and API changes.
…and register in FILES.cmake Create the TCollection_AsciiString.lxx file and include it at the end of TCollection_AsciiString.hxx. Add the new .lxx to FILES.cmake so it is picked up by the build.
…ep inline declarations in header Move numerous small convenience overloads, template-literal helpers and std::string_view wrappers out of the header into TCollection_AsciiString.lxx. Replace their header bodies with inline declarations to centralize implementations, reduce header bloat and keep ABI-friendly inline stubs.
… inline declarations in header Move many AsciiString method/operator implementations out of the header into TCollection_AsciiString.lxx, leaving only inline declarations in the .hxx. Add std::hash specialization in the .lxx and reintroduce operator overloads, literal/template wrappers and forwarding helpers there. Guard all std::string_view-dependent definitions with __cplusplus >= 201703L.
Remove redundant null terminator from AssignCat (reallocate already ensures termination). Fix reallocate to write the terminator at the new length (myString[theLength]) instead of using the stale myLength and adjust the comment accordingly.
dpasukhi
marked this pull request as ready for review
October 19, 2025 16:10
…iiString - include <functional> - Replace undefined pointer comparisons with std::less in AssignCat and Insert to reliably detect self-references into the internal buffer. - Simplify Search and SearchFromEnd loops to for-loops for clarity and maintainability.
AtheneNoctuaPt
approved these changes
Oct 20, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.