Consistent debug/release precondition checks + index_range hardening (#29)

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++

include/compatibility.h

@@ -22,10 +22,30 @@
 
 #pragma once
 
+#include <cstdlib>
+
 #if __cplusplus >= 201703L
 #define CPP17_AVAILABLE
 #endif
 
 #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

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);

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;
     }
 

include/set.h

@@ -388,17 +388,11 @@ namespace fcpp {
                     std::set<UKey> 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<size_t>(vec_size));
             std::set<std::pair<TKey, UKey>> combined_set;
             auto it1 = begin();
             auto it2 = set_begin;

include/vector.h

@@ -212,17 +212,11 @@ namespace fcpp {
                 [previous, vector_copy](const std::function<void(const std::pair<T, U>&)>& 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<void(const std::pair<T, U>&)>& 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<int>({ 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<size_t>(range.end)) {
                 return *this;
             }
             m_vector.erase(begin() + range.start,
@@ -1302,7 +1287,8 @@ namespace fcpp {
         //        shorter_vector -> fcpp::vector<int>({ 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<size_t>(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<size_t>(vec_size));
             std::vector<std::pair<T, U>> 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<size_t>(vec_size) <= size()
+                              && index <= size() - static_cast<size_t>(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<size_t>(vec_size) <= size()
+                              && index <= size() - static_cast<size_t>(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());
         }
     };
 

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)

tests/map_test.cc

@@ -95,6 +95,12 @@ TEST(MapTest, AccessOperator)
     EXPECT_EQ(0, persons["john"]);
 }
 
+TEST(MapTest, AccessConstOperatorMissingKeyDeath)
+{
+    const map<std::string, int> persons({{"jake", 32}, {"mary", 26}, {"david", 40}});
+    EXPECT_DEATH(persons["john"], "");
+}
+
 TEST(MapTest, MapTo)
 {
     const map<std::string, int> persons({{"jake", 32}, {"mary", 26}, {"david", 40}});