Add resize_and_overwrite (reworked #53) - #59
Merged
Conversation
This was referenced Jul 27, 2026
Closed
Following feedback from #49 I've written `resize_and_overwrite` using https://en.cppreference.com/w/cpp/string/basic_string/resize_and_overwrite as a guide.
Reworked the implementation. Three problems with the previous version: 1. op was called as op(p, current_size) instead of op(p, count). The standard passes the requested count, and that is the whole point of the API: the callback needs to know how much writable space it has. Passing the old size silently breaks any callback written against cppreference, with an identical signature. For a size 3 vector resized to 8, libstdc++ passes n=8 while this passed n=3. 2. Shrinking destroyed the tail via destroy_n but left the stored size untouched while op ran. If op threw, ~svector() destroyed those elements a second time (confirmed as a double free under ASan). 3. Growing moved the elements into the new storage and installed it, but left the size at 0 until after op returned. If op threw, every existing element leaked. The rewrite reserves first, commits the shrunken size before calling op, and only then commits op's result, so at every point where op can throw, the stored size describes exactly the live elements. That also makes it much shorter, since reserve() and set_size() already handle the direct/indirect split. Dropped constexpr: svector uses reinterpret_cast, memcpy and placement new, so it can never be constant evaluated, and none of the rest of the API claims constexpr. Added test/unit/resize_and_overwrite.cpp with 11 cases: the op(p, count) contract, growing inside and past the inline capacity, shrinking, returning less than count, resizing to 0, non trivial element types, that no value initialization happens, and both throwing op paths. The Counter helper verifies every element is destroyed exactly once. Five of these fail against the previous implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
martinus
force-pushed
the
pr/53-resize-and-overwrite
branch
from
July 27, 2026 15:42
a88d93a to
69cb2f9
Compare
Contributor
|
Apologies about the exception safety, I tend to work with POD-like types where destruction is trivial and exceptions don't tend to get thrown. |
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.
Reworks #53 by @Andersama. Their original commit is preserved as the first commit here, so merging this closes #53 as merged.
The idea is good and matches where the discussion in #49 landed. The implementation had three defects, all verified by running it.
1.
opreceived the wrong numberstd::string::resize_and_overwrite(count, op)callsop(p, count)— the requested size. The original calledop(p, current_size)— the old size:Same signature, different meaning, no diagnostic. A callback written against the cppreference example the PR cites as its guide would think it has 3 writable slots when it has 8. This is the defect that most needed fixing, since it is silent.
2. Double destruction if
opthrows while shrinkingThe tail was destroyed via
destroy_nbut the stored size was left untouched whileopran, so~svector()destroyed those elements again:3. Full element leak if
opthrows while growingElements were moved into the new storage and
set_indirectwas called, but the size stayed 0 until afteropreturned, so an exception lost all of them:Direct leak of 190 byte(s) in 5 object(s).The rework
reserve()first, commit the shrunken size before callingop, commitop's result after. At every point whereopcan throw, the stored size describes exactly the live elements. This also makes it a lot shorter, sincereserve()and the non-templateset_size()already handle the direct/indirect split.Also dropped
constexpr—svectorusesreinterpret_cast,memcpyand placement new, so it can never be constant-evaluated, and nothing else in the API claims it.Tests
New
test/unit/resize_and_overwrite.cpp, 11 cases: theop(p, count)contract, growth within and past inline capacity, shrinking, returning less thancount, resize to 0, non-trivial element types, that no value-initialization happens, and both throwing-oppaths.Counterverifies every element is destroyed exactly once.5 of the 11 fail against the original implementation (
n == 0gets 10,n == 50gets 2, plusERROR at ~Counter(): got 2 objects still alive!/ SIGABRT), so they are real regression guards rather than decoration.Full suite:
Ok: 2, Fail: 0. Lint clean.Note on scope
resize_and_overwriteisstring-only in C++23, so this is a non-standard extension to a vector either way — that part is your call. If it goes in, it should match the standard signature exactly, which it now does.Depends on #56 for CI; this branch is based on the pre-fix main, so its checks will be red until that merges.
🤖 Generated with Claude Code