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++ 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/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/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 42b5735..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); } @@ -1284,7 +1266,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 +1287,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); @@ -1958,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) { @@ -2019,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); @@ -2037,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, @@ -2047,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/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) 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}});