Skip to content

Commit bfcd227

Browse files
claudeclaude
committed
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.
1 parent c887277 commit bfcd227

5 files changed

Lines changed: 46 additions & 48 deletions

File tree

‎include/compatibility.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,30 @@
2222

2323
#pragma once
2424

25+
#include <cstdlib>
26+
2527
#if __cplusplus >= 201703L
2628
#define CPP17_AVAILABLE
2729
#endif
2830

2931
#if defined(CPP17_AVAILABLE) && !defined(__clang__)
3032
#define PARALLEL_ALGORITHM_AVAILABLE
3133
#endif
34+
35+
// Runtime precondition check that behaves identically in debug and release builds.
36+
// Unlike assert (which is disabled when NDEBUG is defined), this always evaluates
37+
// the condition and calls std::abort() on failure, keeping the library's fatal
38+
// error model consistent across build types.
39+
//
40+
// Define FCPP_NO_PRECONDITION_CHECKS to compile the checks out (e.g. for hot paths
41+
// whose inputs have already been validated).
42+
#ifdef FCPP_NO_PRECONDITION_CHECKS
43+
#define FCPP_PRECONDITION(condition) ((void)0)
44+
#else
45+
#define FCPP_PRECONDITION(condition) \
46+
do { \
47+
if (!(condition)) { \
48+
std::abort(); \
49+
} \
50+
} while (false)
51+
#endif

‎include/map.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,7 @@ class map
656656
const TValue& operator[](const TKey& key) const
657657
{
658658
const auto it = m_map.find(key);
659-
assert(it != end());
659+
FCPP_PRECONDITION(it != end());
660660
return (*it).second;
661661
}
662662

‎include/set.h‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -388,17 +388,11 @@ namespace fcpp {
388388
std::set<UKey> distinct_values(materialized_vector.begin(), materialized_vector.end());
389389
auto it = distinct_values.begin();
390390
previous([&distinct_values, &it, &consumer](const TKey& key) {
391-
if (it == distinct_values.end()) {
392-
assert(false);
393-
std::abort();
394-
}
391+
FCPP_PRECONDITION(it != distinct_values.end());
395392
consumer({key, *it});
396393
++it;
397394
});
398-
if (it != distinct_values.end()) {
399-
assert(false);
400-
std::abort();
401-
}
395+
FCPP_PRECONDITION(it == distinct_values.end());
402396
});
403397
}
404398

@@ -415,17 +409,11 @@ namespace fcpp {
415409
const auto materialized_set = set.get();
416410
size_t index = 0;
417411
previous([&materialized_set, &consumer, &index](const TKey& key) {
418-
if (index >= materialized_set.size()) {
419-
assert(false);
420-
std::abort();
421-
}
412+
FCPP_PRECONDITION(index < materialized_set.size());
422413
consumer({key, materialized_set[index]});
423414
++index;
424415
});
425-
if (index != materialized_set.size()) {
426-
assert(false);
427-
std::abort();
428-
}
416+
FCPP_PRECONDITION(index == materialized_set.size());
429417
});
430418
}
431419

@@ -1169,7 +1157,7 @@ namespace fcpp {
11691157

11701158
void assert_smaller_size(const size_t index) const
11711159
{
1172-
assert(index < size());
1160+
FCPP_PRECONDITION(index < size());
11731161
}
11741162

11751163
#ifdef CPP17_AVAILABLE
@@ -1185,7 +1173,7 @@ namespace fcpp {
11851173
{
11861174
#endif
11871175
const auto vec_size = std::distance(set_begin, set_end);
1188-
assert(size() == vec_size);
1176+
FCPP_PRECONDITION(size() == static_cast<size_t>(vec_size));
11891177
std::set<std::pair<TKey, UKey>> combined_set;
11901178
auto it1 = begin();
11911179
auto it2 = set_begin;

‎include/vector.h‎

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -212,17 +212,11 @@ namespace fcpp {
212212
[previous, vector_copy](const std::function<void(const std::pair<T, U>&)>& consumer) {
213213
size_t index = 0;
214214
previous([&vector_copy, &consumer, &index](const T& element) {
215-
if (index >= vector_copy.size()) {
216-
assert(false);
217-
std::abort();
218-
}
215+
FCPP_PRECONDITION(index < vector_copy.size());
219216
consumer({element, vector_copy[index]});
220217
++index;
221218
});
222-
if (index != vector_copy.size()) {
223-
assert(false);
224-
std::abort();
225-
}
219+
FCPP_PRECONDITION(index == vector_copy.size());
226220
},
227221
capacity_hint);
228222
}
@@ -240,17 +234,11 @@ namespace fcpp {
240234
[previous, vector_copy](const std::function<void(const std::pair<T, U>&)>& consumer) {
241235
size_t index = 0;
242236
previous([&vector_copy, &consumer, &index](const T& element) {
243-
if (index >= vector_copy.size()) {
244-
assert(false);
245-
std::abort();
246-
}
237+
FCPP_PRECONDITION(index < vector_copy.size());
247238
consumer({element, vector_copy[index]});
248239
++index;
249240
});
250-
if (index != vector_copy.size()) {
251-
assert(false);
252-
std::abort();
253-
}
241+
FCPP_PRECONDITION(index == vector_copy.size());
254242
},
255243
capacity_hint);
256244
}
@@ -269,17 +257,11 @@ namespace fcpp {
269257
const auto materialized_vector = vector.get();
270258
size_t index = 0;
271259
previous([&materialized_vector, &consumer, &index](const T& element) {
272-
if (index >= materialized_vector.size()) {
273-
assert(false);
274-
std::abort();
275-
}
260+
FCPP_PRECONDITION(index < materialized_vector.size());
276261
consumer({element, materialized_vector[index]});
277262
++index;
278263
});
279-
if (index != materialized_vector.size()) {
280-
assert(false);
281-
std::abort();
282-
}
264+
FCPP_PRECONDITION(index == materialized_vector.size());
283265
},
284266
capacity_hint);
285267
}
@@ -1962,7 +1944,7 @@ namespace fcpp {
19621944
{
19631945
#endif
19641946
const auto vec_size = std::distance(vec_begin, vec_end);
1965-
assert(m_vector.size() == vec_size);
1947+
FCPP_PRECONDITION(m_vector.size() == static_cast<size_t>(vec_size));
19661948
std::vector<std::pair<T, U>> combined_vector;
19671949
combined_vector.reserve(vec_size);
19681950
for (size_t i = 0; i < vec_size; ++i) {
@@ -2023,7 +2005,8 @@ namespace fcpp {
20232005
const Iterator& vec_end)
20242006
{
20252007
const auto vec_size = std::distance(vec_begin, vec_end);
2026-
assert(index + vec_size >= vec_size && index + vec_size <= size());
2008+
FCPP_PRECONDITION(static_cast<size_t>(vec_size) <= size()
2009+
&& index <= size() - static_cast<size_t>(vec_size));
20272010
std::copy(vec_begin,
20282011
vec_end,
20292012
m_vector.begin() + index);
@@ -2041,7 +2024,8 @@ namespace fcpp {
20412024
const Iterator& vec_end) const
20422025
{
20432026
const auto vec_size = std::distance(vec_begin, vec_end);
2044-
assert(index + vec_size >= vec_size && index + vec_size <= size());
2027+
FCPP_PRECONDITION(static_cast<size_t>(vec_size) <= size()
2028+
&& index <= size() - static_cast<size_t>(vec_size));
20452029
auto replaced_vector(m_vector);
20462030
std::copy(vec_begin,
20472031
vec_end,
@@ -2051,12 +2035,12 @@ namespace fcpp {
20512035

20522036
void assert_smaller_size(size_t index) const
20532037
{
2054-
assert(index < size());
2038+
FCPP_PRECONDITION(index < size());
20552039
}
20562040

20572041
void assert_smaller_or_equal_size(size_t index) const
20582042
{
2059-
assert(index <= size());
2043+
FCPP_PRECONDITION(index <= size());
20602044
}
20612045
};
20622046

‎tests/map_test.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ TEST(MapTest, AccessOperator)
9595
EXPECT_EQ(0, persons["john"]);
9696
}
9797

98+
TEST(MapTest, AccessConstOperatorMissingKeyDeath)
99+
{
100+
const map<std::string, int> persons({{"jake", 32}, {"mary", 26}, {"david", 40}});
101+
EXPECT_DEATH(persons["john"], "");
102+
}
103+
98104
TEST(MapTest, MapTo)
99105
{
100106
const map<std::string, int> persons({{"jake", 32}, {"mary", 26}, {"david", 40}});

0 commit comments

Comments
 (0)