Repository navigation
Add count-value constructors to VariableLengthArray (#63) - #241
Merged
Merged
Conversation
Add `VariableLengthArray(count, value, alloc)`, matching std::vector's
constructor (3), and `VariableLengthArray(count, value, max_size_max,
alloc)`, for both ordinary elements and packed bool. They construct
`count` copies of `value` in exactly the storage needed, throw
std::length_error when `count` exceeds max_size(), and, with exceptions
disabled, truncate to max_size() or to the storage the allocator
provides.
The count's type is deduced (any type convertible to size_type) instead
of being a plain size_type. A plain size_type parameter makes the
existing `VLA<int>({x}, 5, alloc)` ambiguous with the initializer-list
constructor: the braced list prefers initializer_list, but the int
literal 5 converts better to `const int&` than to size_type. A braced
argument cannot be deduced, so that call still selects the
initializer-list constructor. There is no `(count, alloc)` overload,
because `(n, alloc)` already sets max_size_max.
Integer arguments such as `VLA<int>(3, 4, alloc)` used to be ill-formed
and now construct three 4s, as std::vector does; the assign suite's
"non-iterators must not select the range constructor" assertions now
use nullptr_t arguments, which are neither iterators nor counts.
Tests: a new test_variable_length_array_construct suite (int and bool,
checked against std::vector) covers byte boundaries, max_size_max,
oversized counts, count types, and overload resolution against the
max_size, initializer-list, and range constructors, including
size_type and pointer elements. The exception-safety suite covers
throwing copies, allocator construction-hook failures, length_error,
and bad_alloc; the compat suite runs a std::string case against
std::vector; and the fuzzer checks count-value construction with
CopyConstruct.
Closes #63.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db639ba303
ℹ️ 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".
When value_type is itself an input iterator that converts to size_type, `VLA(first, last, alloc)` and `VLA(first, last, max_size_max, alloc)` matched the count-value and range constructor templates equally well and were ambiguous, although they compiled before the count-value constructors existed. std::vector selects its range constructor here. Add a SFINAE-friendly detail::is_input_iterator trait, use it for the existing range-overload constraint, and exclude input iterators from enable_if_count_t, which both count-value overloads in both specializations share. A new test checks range selection against std::vector for such an element type and for packed bool. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Adds the count-value constructors to
cetl::VariableLengthArray, matchingstd::vector::vector(3):Both the general and the packed-
boolversions:countcopies ofvalue, using exactly the storage needed (whole bytes forbool)std::length_errorwhencountis greater thanmax_size(), andstd::bad_allocwhen allocation failsmax_size()or to the storage the allocator provides, which you can detect by comparingsize()withcountConflicts with the existing constructors (the issue asked for this check)
max_size_max. With a plainsize_type countparameter,VLA<int>({x}, 5, alloc)becomes ambiguous. The braced list prefersinitializer_list, but theintliteral5converts better toconst int&than tosize_type. I confirmed the ambiguity with both GCC and Clang. To avoid it, the count's type is deduced, accepting any type convertible tosize_type. A braced argument can't be deduced, so that call still picks the initializer-list constructor.(n, alloc). This still setsmax_size_max. There is nostd::vector-style(count, alloc)overload, because it would collide with that constructor. The docs say so.VLA<std::size_t>andVLA<int*>, still select them.VLA<int>(3, 4, alloc)used to be ill-formed. It now constructs three 4s, asstd::vectordoes. The assign suite's "non-iterators must not select the range constructor" assertions relied on the old behaviour, so they now usenullptr_targuments, which are neither iterators nor counts.Tests
test_variable_length_array_construct.cpp, typed overintandbooland checked againststd::vector. It covers byte boundaries,max_size_max, counts larger thanmax_size(), different count types (including an unscoped enum), and overload resolution against the max-size, initializer-list (parenthesised and braced) and range constructors, withsize_type, pointer andstd::stringelements.test_variable_length_array_exception_safety.cppadds count-value cases for throwing element copies and allocator construction-hook failures, plus a new test forlength_errorandbad_allocthat checks nothing is constructed or allocated.test_variable_length_array_compat.cppadds astd::stringcount-value case that runs against both VLA andstd::vector.CopyConstructoperation now also checks count-value construction against the model. The input format is unchanged.Validation
-fno-exceptions)ts24.4.3), C++14: the construct, exception-safety, compat and assign suites pass with and without exceptions-Wall -Wextra -Werror -Wconversion -Wsign-conversion -pedanticclang-format19.1.7 fromtoolshed:ts24.4.3reports no changes for any line I touched;git diff --checkis clean. Some untouched lines in the compat and exception-safety tests already differ from the format onmain, and I left them alone.Closes #63.
🤖 Generated with Claude Code