From c887277648c5ebce87d9a24471b1ef0b3a04903d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 18:40:12 +0000 Subject: [PATCH 1/3] Harden index_range: const invalid sentinel and overflow-safe range checks (#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. --- include/index_range.h | 2 +- include/vector.h | 8 ++++++-- src/index_range.cc | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/index_range.h b/include/index_range.h index 6670939..fdcc5fc 100644 --- a/include/index_range.h +++ b/include/index_range.h @@ -27,7 +27,7 @@ struct FunctionalCppExport index_range { // Used for returning values of invalid operations - static index_range invalid; + static const index_range invalid; // Create with start index and element count (end index is calculated) static index_range start_count(int start, int count); diff --git a/include/vector.h b/include/vector.h index 42b5735..9f68a00 100644 --- a/include/vector.h +++ b/include/vector.h @@ -1284,7 +1284,10 @@ namespace fcpp { // numbers -> fcpp::vector({ 1, 4, 2, 7, 1 }) vector& remove_range(index_range range) { - if (!range.is_valid || size() < range.end + 1) { + // A valid range guarantees range.end >= 0, so the cast is safe. Comparing + // against range.end directly (rather than range.end + 1) avoids signed + // integer overflow and the signed/unsigned mismatch with size(). + if (!range.is_valid || size() <= static_cast(range.end)) { return *this; } m_vector.erase(begin() + range.start, @@ -1302,7 +1305,8 @@ namespace fcpp { // shorter_vector -> fcpp::vector({ 1, 4, 3, 1, 7, 1 }) [[nodiscard]] vector removing_range(index_range range) const { - if (!range.is_valid || size() < range.end + 1) { + // See remove_range for why the comparison is written this way. + if (!range.is_valid || size() <= static_cast(range.end)) { return *this; } auto shorter_vector(m_vector); diff --git a/src/index_range.cc b/src/index_range.cc index ac8ceb4..1e1d12c 100644 --- a/src/index_range.cc +++ b/src/index_range.cc @@ -22,7 +22,7 @@ #include "index_range.h" -index_range index_range::invalid = start_count(-1, -1); +const index_range index_range::invalid = index_range::start_count(-1, -1); index_range::index_range(int start, int count) : start(-1), end(-1), count(-1) From bfcd227a1cc6d755b16f4edc349e0224154b48f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 18:53:11 +0000 Subject: [PATCH 2/3] Make precondition checks consistent across debug and release (#29) 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. --- include/compatibility.h | 20 ++++++++++++++++++++ include/map.h | 2 +- include/set.h | 24 ++++++----------------- include/vector.h | 42 +++++++++++++---------------------------- tests/map_test.cc | 6 ++++++ 5 files changed, 46 insertions(+), 48 deletions(-) diff --git a/include/compatibility.h b/include/compatibility.h index aba6a3b..4b5faa4 100644 --- a/include/compatibility.h +++ b/include/compatibility.h @@ -22,6 +22,8 @@ #pragma once +#include + #if __cplusplus >= 201703L #define CPP17_AVAILABLE #endif @@ -29,3 +31,21 @@ #if defined(CPP17_AVAILABLE) && !defined(__clang__) #define PARALLEL_ALGORITHM_AVAILABLE #endif + +// Runtime precondition check that behaves identically in debug and release builds. +// Unlike assert (which is disabled when NDEBUG is defined), this always evaluates +// the condition and calls std::abort() on failure, keeping the library's fatal +// error model consistent across build types. +// +// Define FCPP_NO_PRECONDITION_CHECKS to compile the checks out (e.g. for hot paths +// whose inputs have already been validated). +#ifdef FCPP_NO_PRECONDITION_CHECKS +#define FCPP_PRECONDITION(condition) ((void)0) +#else +#define FCPP_PRECONDITION(condition) \ + do { \ + if (!(condition)) { \ + std::abort(); \ + } \ + } while (false) +#endif diff --git a/include/map.h b/include/map.h index c0511cc..0c249cb 100644 --- a/include/map.h +++ b/include/map.h @@ -656,7 +656,7 @@ class map const TValue& operator[](const TKey& key) const { const auto it = m_map.find(key); - assert(it != end()); + FCPP_PRECONDITION(it != end()); return (*it).second; } diff --git a/include/set.h b/include/set.h index b0a5e35..f6ef8f4 100644 --- a/include/set.h +++ b/include/set.h @@ -388,17 +388,11 @@ namespace fcpp { std::set distinct_values(materialized_vector.begin(), materialized_vector.end()); auto it = distinct_values.begin(); previous([&distinct_values, &it, &consumer](const TKey& key) { - if (it == distinct_values.end()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(it != distinct_values.end()); consumer({key, *it}); ++it; }); - if (it != distinct_values.end()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(it == distinct_values.end()); }); } @@ -415,17 +409,11 @@ namespace fcpp { const auto materialized_set = set.get(); size_t index = 0; previous([&materialized_set, &consumer, &index](const TKey& key) { - if (index >= materialized_set.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index < materialized_set.size()); consumer({key, materialized_set[index]}); ++index; }); - if (index != materialized_set.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index == materialized_set.size()); }); } @@ -1169,7 +1157,7 @@ namespace fcpp { void assert_smaller_size(const size_t index) const { - assert(index < size()); + FCPP_PRECONDITION(index < size()); } #ifdef CPP17_AVAILABLE @@ -1185,7 +1173,7 @@ namespace fcpp { { #endif const auto vec_size = std::distance(set_begin, set_end); - assert(size() == vec_size); + FCPP_PRECONDITION(size() == static_cast(vec_size)); std::set> combined_set; auto it1 = begin(); auto it2 = set_begin; diff --git a/include/vector.h b/include/vector.h index 9f68a00..f1b955c 100644 --- a/include/vector.h +++ b/include/vector.h @@ -212,17 +212,11 @@ namespace fcpp { [previous, vector_copy](const std::function&)>& consumer) { size_t index = 0; previous([&vector_copy, &consumer, &index](const T& element) { - if (index >= vector_copy.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index < vector_copy.size()); consumer({element, vector_copy[index]}); ++index; }); - if (index != vector_copy.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index == vector_copy.size()); }, capacity_hint); } @@ -240,17 +234,11 @@ namespace fcpp { [previous, vector_copy](const std::function&)>& consumer) { size_t index = 0; previous([&vector_copy, &consumer, &index](const T& element) { - if (index >= vector_copy.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index < vector_copy.size()); consumer({element, vector_copy[index]}); ++index; }); - if (index != vector_copy.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index == vector_copy.size()); }, capacity_hint); } @@ -269,17 +257,11 @@ namespace fcpp { const auto materialized_vector = vector.get(); size_t index = 0; previous([&materialized_vector, &consumer, &index](const T& element) { - if (index >= materialized_vector.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index < materialized_vector.size()); consumer({element, materialized_vector[index]}); ++index; }); - if (index != materialized_vector.size()) { - assert(false); - std::abort(); - } + FCPP_PRECONDITION(index == materialized_vector.size()); }, capacity_hint); } @@ -1962,7 +1944,7 @@ namespace fcpp { { #endif const auto vec_size = std::distance(vec_begin, vec_end); - assert(m_vector.size() == vec_size); + FCPP_PRECONDITION(m_vector.size() == static_cast(vec_size)); std::vector> combined_vector; combined_vector.reserve(vec_size); for (size_t i = 0; i < vec_size; ++i) { @@ -2023,7 +2005,8 @@ namespace fcpp { const Iterator& vec_end) { const auto vec_size = std::distance(vec_begin, vec_end); - assert(index + vec_size >= vec_size && index + vec_size <= size()); + FCPP_PRECONDITION(static_cast(vec_size) <= size() + && index <= size() - static_cast(vec_size)); std::copy(vec_begin, vec_end, m_vector.begin() + index); @@ -2041,7 +2024,8 @@ namespace fcpp { const Iterator& vec_end) const { const auto vec_size = std::distance(vec_begin, vec_end); - assert(index + vec_size >= vec_size && index + vec_size <= size()); + FCPP_PRECONDITION(static_cast(vec_size) <= size() + && index <= size() - static_cast(vec_size)); auto replaced_vector(m_vector); std::copy(vec_begin, vec_end, @@ -2051,12 +2035,12 @@ namespace fcpp { void assert_smaller_size(size_t index) const { - assert(index < size()); + FCPP_PRECONDITION(index < size()); } void assert_smaller_or_equal_size(size_t index) const { - assert(index <= size()); + FCPP_PRECONDITION(index <= size()); } }; diff --git a/tests/map_test.cc b/tests/map_test.cc index ca55941..b4178db 100644 --- a/tests/map_test.cc +++ b/tests/map_test.cc @@ -95,6 +95,12 @@ TEST(MapTest, AccessOperator) EXPECT_EQ(0, persons["john"]); } +TEST(MapTest, AccessConstOperatorMissingKeyDeath) +{ + const map persons({{"jake", 32}, {"mary", 26}, {"david", 40}}); + EXPECT_DEATH(persons["john"], ""); +} + TEST(MapTest, MapTo) { const map persons({{"jake", 32}, {"mary", 26}, {"david", 40}}); From 469d80ab0a275fc1d9bcf9d5ae7ba54902ca3710 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 19:00:00 +0000 Subject: [PATCH 3/3] docs: document always-on precondition checks and FCPP_NO_PRECONDITION_CHECKS (#29) --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index 2bdf9c9..f703f43 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ The primary focus of this library is * [macOS (Makefiles/g++)](#macos-makefilesg) * [Linux (Makefiles)](#linux-makefiles) * [Windows (Visual Studio)](#windows-visual-studio) +* [Error handling](#error-handling) * [Functional vector usage (fcpp::vector)](#functional-vector-usage-fcppvector) * [extract unique (distinct) elements in a set](#extract-unique-distinct-elements-in-a-set) * [zip, map, filter, sort, reduce](#zip-map-filter-sort-reduce) @@ -91,6 +92,17 @@ cmake -S . -B build ``` Then open the generated ```functional_cpp.sln``` in the ```build``` folder. +## Error handling +Operations with a precondition (for example subscripting with ```operator[]```, ```replace_range_at```, or ```zip``` on containers of unequal size) validate that precondition at runtime. If it is violated, the program is terminated immediately via ```std::abort()```. + +Unlike the standard library's ```assert```, these checks are **always active and behave identically in debug and release builds** (they are not disabled by ```NDEBUG```), so an out-of-bounds access fails fast in production instead of becoming silent undefined behavior. + +If you have a performance-critical section whose inputs are already known to be valid, you can compile the checks out by defining ```FCPP_NO_PRECONDITION_CHECKS```: +```console +cmake -S . -B build -DCMAKE_CXX_FLAGS="-DFCPP_NO_PRECONDITION_CHECKS" +``` +With the checks disabled, violating a precondition is undefined behavior, exactly like the underlying ```std::vector```/```std::set```/```std::map```. + ## Functional vector usage (fcpp::vector) ### extract unique (distinct) elements in a set ```c++