You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
testing inplace_vector with newer compiler version
also running Asan and Tsan for all buils feels wasteful and hella slow, furthermore I dont really see a point of running Tsan for inplace_vector at all...
according to opus 4.7 previously mentioned gcc15 warning is false positive,
That's GCC 15's -Wstringop-overread firing on erase(first, last) at include/beman/inplace_vector/inplace_vector.hpp:382. With N=1, the storage is one element wide (4 or 8 bytes), but GCC can't prove last - first <= size() so it assumes the std::move source range could be up to 1016 bytes — hence "reading between 5 and 1016 bytes from a region of size 4."
Ranked, with the tradeoff that drives each choice:
For: Encodes the standard's existing precondition in the API contract. Fixes the warning for all callers. Lets the optimizer use the bound. Spec-clean — only formalizes UB the standard already grants.
Against: Buggy precondition arithmetic could be silently optimized away. Couples to compiler analysis quality.
Pick when: You see this as a library contract issue.
For: Zero codegen change, zero new UB, no optimizer coupling. Honest "known false positive" acknowledgment. Fixes for all callers.
Against: GCC-specific (guard with #if defined(__GNUC__) && !defined(__clang__)). Disables the diagnostic at this site forever.
Pick when: You'd rather not encode assumptions.
3. Localized #pragma in the test
For: Header stays clean. Suppression co-located with the only known trigger.
Against: Whack-a-mole — every future N=1 erase caller will see it again.
Pick when: You're sure this is a one-off and don't want to touch the header.
4. Hand-rolled move loop
for (iterator src = f + (last-first), dst = f; src != end(); ++src, ++dst)
*dst = std::move(*src);
For: No pragma, no [[assume]], no UB strengthening, no optimizer coupling.
Against: Loses std::move's memmove specialization for trivial T (usually re-vectorized, not guaranteed).
Pick when: You want zero compiler-specific knobs and accept potential minor perf cost.
Recommendation
Default to #1 ([[assume]] in header) — only fix that documents the contract, helps the optimizer, aligns with the standard, and works for every caller.
Fall back to #2 (header pragma) to avoid assumption hints. Use #3 (test pragma) only if the library shouldn't change. Reach for #4 (loop) if opposed to compiler-specific knobs.
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
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.
testing inplace_vector with newer compiler version
also running Asan and Tsan for all buils feels wasteful and hella slow, furthermore I dont really see a point of running Tsan for inplace_vector at all...