Consistent debug/release precondition checks + index_range hardening (#29)#33
Merged
Conversation
…ecks (#29) Make index_range::invalid a const static so the shared sentinel cannot be mutated by callers. In vector::remove_range / removing_range, compare the vector size against range.end directly instead of `range.end + 1`. A valid range guarantees range.end >= 0, so casting to size_t is safe; this removes the potential signed-integer overflow at INT_MAX and the signed/unsigned comparison between size() and an int. Behavior is unchanged for all valid ranges. Note: the assert/abort-on-misuse contract (e.g. replace_range_at bounds, zip size mismatches) is intentionally left as-is; it is the library's documented, death-tested error model and changing it to exceptions is a separate decision.
The library guarded out-of-bounds and size-mismatch operations with assert, which the standard disables when NDEBUG is defined. As a result the checks fired (aborting) in debug but vanished in release, turning operator[] and replace_range_at into silent undefined behavior / buffer overflows there. Introduce FCPP_PRECONDITION in compatibility.h: an always-on check that evaluates its condition and calls std::abort() on failure in every build, preserving the library's existing fatal error model. Replace the assert / "assert(false); std::abort();" precondition checks in vector, set and map with it. Define FCPP_NO_PRECONDITION_CHECKS to compile the checks out for hot paths whose inputs are already validated. Debug and release now behave identically; all existing EXPECT_DEATH tests pass under both build types. Added a death test for map's const operator[] with a missing key, which previously had no coverage.
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
2 participants
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.
Summary
Addresses #29. The unifying theme is making the library behave consistently between debug and release builds, plus the smaller
index_rangehardening from the same issue.1. Consistent precondition checks across debug and release
Previously every safety check was an
assert, which the standard disables whenNDEBUGis defined. So checks fired (aborting) in debug but vanished in release, turningoperator[]andreplace_range_atinto silent undefined behavior / buffer overflows there.Introduced
FCPP_PRECONDITIONincompatibility.h— an always-on check that evaluates its condition and callsstd::abort()on failure in every build, preserving the library's existing fatal error model:All
assert/assert(false); std::abort();precondition checks invector,set, andmapwere converted to it (bounds checks,replace_range_atbounds,zipsize-mismatch,map's constoperator[]missing-key). DefineFCPP_NO_PRECONDITION_CHECKSto compile the checks out for hot paths whose inputs are already validated.operator[]out of boundsabort(same as debug)replace_range_atoversizedabort(same as debug)zipsize mismatchreplace_range_at's bounds expression was also rewritten to be overflow-safe (vec_size <= size() && index <= size() - vec_size).2.
index_rangehardeningindex_range::invalidis nowconst, so the shared sentinel can't be mutated by callers.remove_range/removing_rangecomparesize()againstrange.enddirectly instead ofrange.end + 1, removing a potential signed-overflow atINT_MAXand thesize_t/intmismatch. Behavior is unchanged for all valid ranges.3. Docs
Added an "Error handling" section to the README documenting the always-on checks and the
FCPP_NO_PRECONDITION_CHECKSopt-out.Tests
MapTest.AccessConstOperatorMissingKeyDeath(the constoperator[]missing-key path previously had no coverage).EXPECT_DEATHtests now pass under release as well as debug.Verification
Built and ran the full suite under Debug and Release × C++11 and C++17 — all green (301 tests in C++17). The
FCPP_NO_PRECONDITION_CHECKSopt-out path also compiles cleanly.Note on scope
This intentionally keeps the library's fatal-abort error model (the maintainer's existing, death-test-locked choice) rather than switching to exceptions — it just makes that model consistent across build types. Switching to exceptions would be a separate, larger decision.
Closes #29.
https://claude.ai/code/session_011y3XcVg3UCcgi5JMEo2tso
Generated by Claude Code