Fix 32-bit compilation errors in TextView.h by removing ambiguous constructor overloads and revised TextView.cc header includes#13185
Open
grahamsedman wants to merge 2 commits into
Conversation
Add missing standard library headers required for compilation, such as <algorithm>, <cstring>, <cctype>, <strings.h>, <sys/types.h>, <ostream>, and <limits>. Reorder existing includes for better organisation. Apply consistent indentation and formatting style (placing opening braces on new lines) to the entire file to improve readability and adherence to style guidelines. Remove redundant TextView constructor overloads taking unsigned and ssize_t to simplify the interface and reduce ambiguity. Update namespace closing comments to reflect the new brace style.
Update the source code style and remove unnecessary header dependencies. * Adjust indentation for the contents of the swoc and SWOC_VERSION_NS namespaces. * Move opening braces for functions and namespaces to new lines. * Remove unused <cctype> and <sstream> includes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses 32-bit (ILP32) build failures in SWOC’s TextView by removing constructor overloads that become duplicate signatures when size_t == unsigned and ssize_t == int (e.g., arm-linux-gnueabihf). It also aligns TextView.cc to rely on the headers included by TextView.h.
Changes:
- Removed the
TextView(char const *, unsigned)andTextView(char const *, ssize_t)constructor overloads, retaining onlysize_tandintforms to avoid 32-bit redeclaration errors. - Updated/expanded
TextView.hstandard library includes and adjustedTextView.ccincludes accordingly. - Applied clang-format–driven formatting updates (notably namespace/brace layout).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/swoc/src/TextView.cc | Drops redundant includes and relies on TextView.h for required standard headers; formatting updates only. |
| lib/swoc/include/swoc/TextView.h | Removes ambiguous constructor overloads causing 32-bit signature collisions; updates includes and reformats inline implementations. |
Comment on lines
+1129
to
+1130
| // @internal If there is more than one overload for numeric types, it's easy to get ambiguity. The only | ||
| // fix, unfortunately, is lots of overloads to cover the ambiguous cases. |
| * is @c nullptr the length is 0. Otherwise @c strlen is used to calculate the length. | ||
| */ | ||
| constexpr TextView(char const *ptr, int n) noexcept; | ||
|
|
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.
This PR fixes build failures occurring on 32-bit architectures (specifically
arm-linux-gnueabihf).Problem
On 32-bit systems,
size_tis defined asunsigned intandssize_tis defined asint. TheTextViewclass currently defines distinct constructor overloads forsize_t,unsigned,ssize_t, andint.This results in duplicate function signatures on 32-bit builds, causing the compiler to throw
constructor cannot be redeclarederrors.Errors observed:
Solution
To resolve this ambiguity, this PR refactors the constructor overloads to rely only on
size_tandint.TextView(char const *ptr, unsigned n)TextView(char const *ptr, ssize_t n)TextView(char const *ptr, size_t n)(Coversunsignedon 32-bit andsize_ton 64-bit)TextView(char const *ptr, int n)(Coversssize_ton 32-bit andinton 64-bit)The logic within the retained constructors (specifically handling
n < 0forint) remains unchanged, ensuring functional parity across architectures.Changes
lib/swoc/include/swoc/TextView.h:Checklist:
TextView.handTextView.ccTextView.ccto the ones used in theTextView.hheader fileCompilation Errors
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:122:13: error: constructor cannot be redeclared
122 | constexpr TextView(char const *ptr, unsigned n) noexcept;
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:115:13: note: previous declaration is here
115 | constexpr TextView(char const *ptr, size_t n) noexcept;
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:142:13: error: constructor cannot be redeclared
142 | constexpr TextView(char const *ptr, int n) noexcept;
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:132:13: note: previous declaration is here
132 | constexpr TextView(char const *ptr, ssize_t n) noexcept;
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:1128:28: error: redefinition of 'TextView'
1128 | inline constexpr TextView::TextView(const char *ptr, unsigned n) noexcept : super_type(ptr, size_t(n)) {}
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:1126:28: note: previous definition is here
1126 | inline constexpr TextView::TextView(const char *ptr, size_t n) noexcept
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:1131:28: error: redefinition of 'TextView'
1131 | inline constexpr TextView::TextView(const char *ptr, int n) noexcept
| ^
/home/grahamsedman/Projects/trafficserver/lib/swoc/include/swoc/TextView.h:1129:28: note: previous definition is here
1129 | inline constexpr TextView::TextView(const char *ptr, ssize_t n) noexcept