Repository navigation
Add ordering comparison operators to VariableLengthArray (#67) - #243
Conversation
VLA had only == and !=. Add lexicographic ordering for both ordinary
elements and packed bool, matching std::vector in each language
standard:
- Before C++20: operator<, <=, > and >=. operator< uses
std::lexicographical_compare, and the other three are derived from it
as std::vector's are, so the element type needs only operator<.
- From C++20 (when __cpp_lib_three_way_comparison is available):
operator<=>, built on the standard's synth-three-way. It uses the
element's own <=>, or a weak ordering from operator< when the element
has no <=>. The compiler rewrites <, <=, > and >= from it, as for
std::vector. The result category follows the element's: strong for
int, partial for double, weak for operator<-only types. An element
type with no ordering stays usable, and the array is not three-way
comparable.
The two definitions are kept separate deliberately. For partially
ordered elements std::vector's answers differ between the standards:
with NaN, {NaN, 1} < {NaN, 2} is true before C++20 and false after, and
<= and >= differ too. Each path matches std::vector for its standard.
The packed-bool operators compare bits through a shared
first_difference() helper and are noexcept. As for std::vector, neither
max_size() nor capacity affects the result.
A new test_variable_length_array_compare suite (int and bool) checks all
six operators against std::vector over every pair from a set of
sequences: all bit patterns up to five elements, plus longer runs that
differ at the first, middle or last element and cross packed-byte
boundaries. It also covers shorter prefixes, bounds and capacity, NaN
pairs where the standards disagree, operator<-only and unordered element
types, and noexcept. Under C++20 it checks <=> against std::vector,
including the result category and three_way_comparable.
Closes #67.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd29cbb912
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
detail::SynthThreeWay only required `lhs < rhs` and `rhs < lhs` to compile, whereas the standard's synth-three-way also requires both results to be boolean-testable. If an element's operator< returned void, the constraint passed, and deducing the return type then instantiated `if (lhs < rhs)`, a hard error even inside a requires-expression. If the result had only an explicit operator bool, VLA<T> had an operator<=> that std::vector<T> does not. Add the standard's exposition-only boolean-testable as detail::BooleanTestable and constrain both expressions with it, so operator<=> drops out of overload resolution exactly when std::vector's does. New C++20 checks cover void and explicit-bool operator< against std::vector; the previous header fails them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Two gaps found by measuring C++20 coverage of the new code: - With double elements, SynthThreeWay only ever produced equivalent or unordered results, because the existing data reaches NaN as soon as the first elements match. Compare a set of ordinary doubles against std::vector's <=>, including -0.0 against 0.0. - Nothing exercised the second half of BooleanTestable, which requires the negation of operator<'s result to convert to bool as well. Add an element whose result converts to bool but has a void operator!, and check that, like std::vector, the array then has no operator<=>. Removing the negation requirement from the header makes these assertions fail. The only part of the C++20 code still not fully covered is the equal-bits branch of the bool operator<=>, which cannot happen because first_difference() only stops at differing bits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Adds ordering comparisons to
cetl::VariableLengthArray, matchingstd::vector's comparison operators. VLA already had==and!=.<,<=,>,>=std::lexicographical_compare; the element needs onlyoperator<__cpp_lib_three_way_comparison)<=>(the compiler rewrites<,<=,>,>=from it)<=>, or a weak ordering fromoperator<Both the general and the packed-
boolversions are covered. Thebooloperators compare bits through a sharedfirst_difference()helper and arenoexcept. As forstd::vector,max_size()and capacity don't affect the result.Why two definitions
std::vector's answers differ between C++17 and C++20 when elements are only partially ordered, asdoubleis with NaN:{NaN, 1} < {NaN, 2}is true before C++20, because an unordered pair counts as equivalent and comparison moves on to the next element. It is false in C++20, because<=>stops at the first unordered pair.{1, NaN} <= {1, 2}is true before C++20 (!(b < a)) and false in C++20.The first version, with only the C++17 operators, matched
std::vectorat C++14 but failed this comparison againststd::vectorin a C++20 build. So each standard gets the definitionstd::vectoruses there.C++20 details
std::strong_orderingforintandbool,std::partial_orderingfordouble, andstd::weak_orderingfor a type with onlyoperator<.operator<=>is a constrained member template. An element type with no ordering can still be stored, andstd::three_way_comparable<VLA<T>>is then false rather than a compile error.<compare>in CETL. It is only included when compiling as C++20 or later.Tests
New
test_variable_length_array_compare.cpp, typed overintandbool:std::vectorfor every pair from about 100 sequences: every bit pattern up to five elements, plus runs of 7, 8, 9, 16 and 17 elements differing at the first, middle or last element. Those cross packed-byte boundaries forbool.intvalues.std::vectorin whichever standard the test is built with.operator<, and element types with no ordering at all (the array is still usable).noexceptfor thebooloperators.<=>againststd::vectorfor every pair, the result category forint/bool/double/operator<-only elements, andstd::three_way_comparable.Validation
-fno-exceptions)<=>tests-Wall -Wextra -Werror -Wconversion -Wsign-conversion -Wfloat-equal -pedanticts24.4.3: the compare suite compiles cleanly with GCC 7 (C++14), GCC 13 (C++20) and Clang 19 (C++20)clang-format19.1.7 fromtoolshed:ts24.4.3reports no changes;git diff --checkis cleanCloses #67.
🤖 Generated with Claude Code