From 2f699d2cdb23efb1550ad5f6b3a7602e5bdeeec1 Mon Sep 17 00:00:00 2001 From: John Steele Scott Date: Thu, 25 Feb 2021 21:27:59 +1030 Subject: [PATCH 1/5] Don't include . (#2148) This commit replaces use of the assert() macro in format-inl.h with FMT_ASSERT(). This allows us to drop the cassert include. --- include/fmt/format-inl.h | 15 +++++++-------- test/test-assert.h | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/include/fmt/format-inl.h b/include/fmt/format-inl.h index 2bc5a9ac1103..c5fa83bf1218 100644 --- a/include/fmt/format-inl.h +++ b/include/fmt/format-inl.h @@ -9,7 +9,6 @@ #define FMT_FORMAT_INL_H_ #include -#include #include #include #include @@ -148,7 +147,7 @@ FMT_FUNC void format_error_code(detail::buffer& out, int error_code, if (message.size() <= inline_buffer_size - error_code_size) format_to(it, FMT_STRING("{}{}"), message, SEP); format_to(it, FMT_STRING("{}{}"), ERROR_STR, error_code); - assert(out.size() <= inline_buffer_size); + FMT_ASSERT(out.size() <= inline_buffer_size, "must not require dynamic alloc"); } FMT_FUNC void report_error(format_func func, int error_code, @@ -1219,7 +1218,7 @@ struct accumulator { if (lower < n) ++upper; } void operator>>=(int shift) { - assert(shift == 32); + FMT_ASSERT(shift == 32, "only 32-bit shift is supported"); (void)shift; lower = (upper << 32) | (lower >> 32); upper >>= 32; @@ -1298,7 +1297,7 @@ class bigint { public: bigint() : exp_(0) {} explicit bigint(uint64_t n) { assign(n); } - ~bigint() { assert(bigits_.capacity() <= bigits_capacity); } + ~bigint() { FMT_ASSERT(bigits_.capacity() <= bigits_capacity, ""); } bigint(const bigint&) = delete; void operator=(const bigint&) = delete; @@ -1324,7 +1323,7 @@ class bigint { int num_bigits() const { return static_cast(bigits_.size()) + exp_; } FMT_NOINLINE bigint& operator<<=(int shift) { - assert(shift >= 0); + FMT_ASSERT(shift >= 0, "shift must be non-negative"); exp_ += shift / bigit_bits; shift %= bigit_bits; if (shift == 0) return *this; @@ -1386,7 +1385,7 @@ class bigint { // Assigns pow(10, exp) to this bigint. void assign_pow10(int exp) { - assert(exp >= 0); + FMT_ASSERT(exp >= 0, "exponent must be non-negative"); if (exp == 0) return assign(1); // Find the top bit. int bitmask = 1; @@ -2557,11 +2556,11 @@ int snprintf_float(T value, int precision, float_specs specs, --exp_pos; } while (*exp_pos != 'e'); char sign = exp_pos[1]; - assert(sign == '+' || sign == '-'); + FMT_ASSERT(sign == '+' || sign == '-', "expect valid sign character"); int exp = 0; auto p = exp_pos + 2; // Skip 'e' and sign. do { - assert(is_digit(*p)); + FMT_ASSERT(is_digit(*p), "expect digit"); exp = exp * 10 + (*p++ - '0'); } while (p != end); if (sign == '-') exp = -exp; diff --git a/test/test-assert.h b/test/test-assert.h index f613722ffa4a..fe949720025f 100644 --- a/test/test-assert.h +++ b/test/test-assert.h @@ -22,8 +22,24 @@ class assertion_failure : public std::logic_error { void assertion_failure::avoid_weak_vtable() {} +inline void throw_assertion_failure (const char *message) +{ +# if FMT_GCC_VERSION >= 600 + // Avoid warnings when FMT_ASSERT is used in a destructor. +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wterminate" +# endif + + throw assertion_failure(message); + +# if FMT_GCC_VERSION >= 600 +# pragma GCC diagnostic pop +# endif +} + + #define FMT_ASSERT(condition, message) \ - if (!(condition)) throw assertion_failure(message); + if (!(condition)) throw_assertion_failure(message); // Expects an assertion failure. #define EXPECT_ASSERT(stmt, message) \ From 131e5843912993ea17024fdca274e74c78d9148a Mon Sep 17 00:00:00 2001 From: John Steele Scott Date: Thu, 25 Feb 2021 22:39:02 +1030 Subject: [PATCH 2/5] FMT_GCC_VERSION is not defined when we include test-assert.h, use __GCC__ instead. --- test/test-assert.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test-assert.h b/test/test-assert.h index fe949720025f..d91a9ccd4a4a 100644 --- a/test/test-assert.h +++ b/test/test-assert.h @@ -24,7 +24,7 @@ void assertion_failure::avoid_weak_vtable() {} inline void throw_assertion_failure (const char *message) { -# if FMT_GCC_VERSION >= 600 +# if defined(__GNUC__) && __GNUC__ >= 6 // Avoid warnings when FMT_ASSERT is used in a destructor. # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wterminate" @@ -32,7 +32,7 @@ inline void throw_assertion_failure (const char *message) throw assertion_failure(message); -# if FMT_GCC_VERSION >= 600 +# if defined(__GNUC__) && __GNUC__ >= 6 # pragma GCC diagnostic pop # endif } From 4471b2f1335002a1fa3f0166ad8d21ff0efd4ce0 Mon Sep 17 00:00:00 2001 From: John Steele Scott Date: Mon, 1 Mar 2021 20:34:06 +1030 Subject: [PATCH 3/5] Don't explicitly suppress GCC's -Wterminate in tests' FMT_ASSERT. Throwing from a separate function is enough to silence the warning, no need to explicitly suppress it. --- test/test-assert.h | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/test/test-assert.h b/test/test-assert.h index d91a9ccd4a4a..42844a058450 100644 --- a/test/test-assert.h +++ b/test/test-assert.h @@ -22,19 +22,11 @@ class assertion_failure : public std::logic_error { void assertion_failure::avoid_weak_vtable() {} +// We use a separate function (rather than throw directly from FMT_ASSERT) to +// avoid GCC's -Wterminate warning when FMT_ASSERT is used in a destructor. inline void throw_assertion_failure (const char *message) { -# if defined(__GNUC__) && __GNUC__ >= 6 - // Avoid warnings when FMT_ASSERT is used in a destructor. -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wterminate" -# endif - throw assertion_failure(message); - -# if defined(__GNUC__) && __GNUC__ >= 6 -# pragma GCC diagnostic pop -# endif } From 2f7a6d87f6ffbd724437cb78ccf2dc3fbfd52df8 Mon Sep 17 00:00:00 2001 From: John Steele Scott Date: Mon, 1 Mar 2021 20:38:22 +1030 Subject: [PATCH 4/5] Remove messages from assertions added in 2f699d2. --- include/fmt/format-inl.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/fmt/format-inl.h b/include/fmt/format-inl.h index c5fa83bf1218..f6573cb91d36 100644 --- a/include/fmt/format-inl.h +++ b/include/fmt/format-inl.h @@ -147,7 +147,7 @@ FMT_FUNC void format_error_code(detail::buffer& out, int error_code, if (message.size() <= inline_buffer_size - error_code_size) format_to(it, FMT_STRING("{}{}"), message, SEP); format_to(it, FMT_STRING("{}{}"), ERROR_STR, error_code); - FMT_ASSERT(out.size() <= inline_buffer_size, "must not require dynamic alloc"); + FMT_ASSERT(out.size() <= inline_buffer_size, ""); } FMT_FUNC void report_error(format_func func, int error_code, @@ -1218,7 +1218,7 @@ struct accumulator { if (lower < n) ++upper; } void operator>>=(int shift) { - FMT_ASSERT(shift == 32, "only 32-bit shift is supported"); + FMT_ASSERT(shift == 32, ""); (void)shift; lower = (upper << 32) | (lower >> 32); upper >>= 32; @@ -1323,7 +1323,7 @@ class bigint { int num_bigits() const { return static_cast(bigits_.size()) + exp_; } FMT_NOINLINE bigint& operator<<=(int shift) { - FMT_ASSERT(shift >= 0, "shift must be non-negative"); + FMT_ASSERT(shift >= 0, ""); exp_ += shift / bigit_bits; shift %= bigit_bits; if (shift == 0) return *this; @@ -1385,7 +1385,7 @@ class bigint { // Assigns pow(10, exp) to this bigint. void assign_pow10(int exp) { - FMT_ASSERT(exp >= 0, "exponent must be non-negative"); + FMT_ASSERT(exp >= 0, ""); if (exp == 0) return assign(1); // Find the top bit. int bitmask = 1; @@ -2556,11 +2556,11 @@ int snprintf_float(T value, int precision, float_specs specs, --exp_pos; } while (*exp_pos != 'e'); char sign = exp_pos[1]; - FMT_ASSERT(sign == '+' || sign == '-', "expect valid sign character"); + FMT_ASSERT(sign == '+' || sign == '-', ""); int exp = 0; auto p = exp_pos + 2; // Skip 'e' and sign. do { - FMT_ASSERT(is_digit(*p), "expect digit"); + FMT_ASSERT(is_digit(*p), ""); exp = exp * 10 + (*p++ - '0'); } while (p != end); if (sign == '-') exp = -exp; From 0b8276f835d33a8fe193c2a6190d601758b7e7cb Mon Sep 17 00:00:00 2001 From: John Steele Scott Date: Wed, 3 Mar 2021 19:43:04 +1030 Subject: [PATCH 5/5] Correct formatting around throw_assertion_failure(). --- test/test-assert.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/test-assert.h b/test/test-assert.h index 42844a058450..3406e1ba3ddb 100644 --- a/test/test-assert.h +++ b/test/test-assert.h @@ -24,12 +24,10 @@ void assertion_failure::avoid_weak_vtable() {} // We use a separate function (rather than throw directly from FMT_ASSERT) to // avoid GCC's -Wterminate warning when FMT_ASSERT is used in a destructor. -inline void throw_assertion_failure (const char *message) -{ +inline void throw_assertion_failure(const char* message) { throw assertion_failure(message); } - #define FMT_ASSERT(condition, message) \ if (!(condition)) throw_assertion_failure(message);