From d0b2d40b5509259656a8c125e3f1501b6f949f12 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Sat, 22 Jun 2024 20:50:54 +0100 Subject: [PATCH] annotate handle_error with [[noreturn]] --- changelog/current.md | 9 +++- src/c4/enum.hpp | 1 - src/c4/error.cpp | 28 +++++++------ src/c4/error.hpp | 2 +- src/c4/memory_resource.cpp | 2 - src/c4/szconv.hpp | 2 +- test/c4/libtest/supprwarn_push.hpp | 1 + test/c4/libtest/test.cpp | 4 ++ test/c4/test.hpp | 67 +++++++++++++++++++++++------- test/test_error.cpp | 33 +++------------ test/test_error_exception.cpp | 1 + test/test_memory_resource.cpp | 28 +++++++------ test/test_szconv.cpp | 10 +++-- 13 files changed, 111 insertions(+), 77 deletions(-) diff --git a/changelog/current.md b/changelog/current.md index e2d8d5a2..10cdc1fd 100644 --- a/changelog/current.md +++ b/changelog/current.md @@ -1,4 +1,11 @@ - Fix [rapidyaml#445](https://github.com/biojppm/biojppm/pull/445): Amalgamate: fix include of ``. -- Add `bool from_chars(csubstr s, fmt::overflow_checked_ *wrapper)`. There was already is a function receiving `&wrapper`, but `*wrapper` was missing for use with generic code. +- Add `C4_MINGW` ([PR#139](https://github.com/biojppm/c4core/pull/139)) +- Annotate `c4::handle_error()` with `[[noreturn]]` ([PR#137](https://github.com/biojppm/c4core/pull/137)) +- Add `bool from_chars(csubstr s, fmt::overflow_checked_ *wrapper)`. There was already a function receiving `&wrapper`, but `*wrapper` was missing for use with generic code. - Update fast_float to v6.1.1 ([PR#136](https://github.com/biojppm/c4core/pull/136)) + + +### Thanks + +- @toge diff --git a/src/c4/enum.hpp b/src/c4/enum.hpp index 47720d1a..ae12f51b 100644 --- a/src/c4/enum.hpp +++ b/src/c4/enum.hpp @@ -139,7 +139,6 @@ size_t eoffs(EnumOffsetType which) } default: C4_ERROR("unknown offset type %d", (int)which); - return 0; } } diff --git a/src/c4/error.cpp b/src/c4/error.cpp index de28fd88..32a266d3 100644 --- a/src/c4/error.cpp +++ b/src/c4/error.cpp @@ -49,6 +49,7 @@ namespace c4 { static error_flags s_error_flags = ON_ERROR_DEFAULTS; static error_callback_type s_error_callback = nullptr; + //----------------------------------------------------------------------------- error_flags get_error_flags() @@ -70,6 +71,7 @@ void set_error_callback(error_callback_type cb) s_error_callback = cb; } + //----------------------------------------------------------------------------- void handle_error(srcloc where, const char *fmt, ...) @@ -102,23 +104,24 @@ void handle_error(srcloc where, const char *fmt, ...) { if(s_error_callback) { - s_error_callback(buf, msglen/*ss.c_strp(), ss.tellp()*/); + s_error_callback(buf, msglen); } } - if(s_error_flags & ON_ERROR_ABORT) - { - abort(); - } - if(s_error_flags & ON_ERROR_THROW) { #if defined(C4_EXCEPTIONS_ENABLED) && defined(C4_ERROR_THROWS_EXCEPTION) throw std::runtime_error(buf); -#else - abort(); #endif } + + if(s_error_flags & ON_ERROR_ABORT) + { + abort(); + } + + abort(); // abort anyway, in case nothing was set + C4_UNREACHABLE_AFTER_ERR(); } //----------------------------------------------------------------------------- @@ -126,20 +129,19 @@ void handle_error(srcloc where, const char *fmt, ...) void handle_warning(srcloc where, const char *fmt, ...) { va_list args; - char buf[1024]; //sstream ss; + char buf[1024]; va_start(args, fmt); vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); C4_LOGF_WARN("\n"); #if defined(C4_ERROR_SHOWS_FILELINE) && defined(C4_ERROR_SHOWS_FUNC) - C4_LOGF_WARN("%s:%d: WARNING: %s\n", where.file, where.line, buf/*ss.c_strp()*/); + C4_LOGF_WARN("%s:%d: WARNING: %s\n", where.file, where.line, buf); C4_LOGF_WARN("%s:%d: WARNING: here: %s\n", where.file, where.line, where.func); #elif defined(C4_ERROR_SHOWS_FILELINE) - C4_LOGF_WARN("%s:%d: WARNING: %s\n", where.file, where.line, buf/*ss.c_strp()*/); + C4_LOGF_WARN("%s:%d: WARNING: %s\n", where.file, where.line, buf); #elif ! defined(C4_ERROR_SHOWS_FUNC) - C4_LOGF_WARN("WARNING: %s\n", buf/*ss.c_strp()*/); + C4_LOGF_WARN("WARNING: %s\n", buf); #endif - //c4::log.flush(); } //----------------------------------------------------------------------------- diff --git a/src/c4/error.hpp b/src/c4/error.hpp index 93fa8b60..562b1d87 100644 --- a/src/c4/error.hpp +++ b/src/c4/error.hpp @@ -177,7 +177,7 @@ struct ScopedErrorSettings /** source location */ struct srcloc; -C4CORE_EXPORT void handle_error(srcloc s, const char *fmt, ...); +C4CORE_EXPORT [[noreturn]] void handle_error(srcloc s, const char *fmt, ...); C4CORE_EXPORT void handle_warning(srcloc s, const char *fmt, ...); diff --git a/src/c4/memory_resource.cpp b/src/c4/memory_resource.cpp index cec7efa2..ead3eddd 100644 --- a/src/c4/memory_resource.cpp +++ b/src/c4/memory_resource.cpp @@ -207,7 +207,6 @@ void* MemoryResourceLinear::do_allocate(size_t sz, size_t alignment, void *hint) if(m_pos + sz > m_size) { C4_ERROR("out of memory"); - return nullptr; } void *mem = m_mem + m_pos; size_t space = m_size - m_pos; @@ -222,7 +221,6 @@ void* MemoryResourceLinear::do_allocate(size_t sz, size_t alignment, void *hint) else { C4_ERROR("could not align memory"); - mem = nullptr; } return mem; } diff --git a/src/c4/szconv.hpp b/src/c4/szconv.hpp index 9d0c4786..e571f9f3 100644 --- a/src/c4/szconv.hpp +++ b/src/c4/szconv.hpp @@ -53,7 +53,7 @@ szconv(SizeIn sz) noexcept template C4_ALWAYS_INLINE typename std::enable_if::value, SizeOut>::type -szconv(SizeIn sz) C4_NOEXCEPT_X +szconv(SizeIn sz) { C4_XASSERT(sz >= 0); C4_XASSERT_MSG((SizeIn)sz <= (SizeIn)std::numeric_limits::max(), "size conversion overflow: in=%zu", (size_t)sz); diff --git a/test/c4/libtest/supprwarn_push.hpp b/test/c4/libtest/supprwarn_push.hpp index 1c9fb18b..fe373552 100644 --- a/test/c4/libtest/supprwarn_push.hpp +++ b/test/c4/libtest/supprwarn_push.hpp @@ -44,6 +44,7 @@ # pragma warning(disable:4127) // conditional expression is constant # pragma warning(disable:4189) // local variable is initialized but not referenced # pragma warning(disable:4389) // '==': signed/unsigned mismatch +# pragma warning(disable:4611) // interaction between '_setjmp' and C++ object destruction is non-portable # pragma warning(disable:4702) // unreachable code #endif diff --git a/test/c4/libtest/test.cpp b/test/c4/libtest/test.cpp index f005d24a..18046d0d 100644 --- a/test/c4/libtest/test.cpp +++ b/test/c4/libtest/test.cpp @@ -3,5 +3,9 @@ namespace c4 { size_t TestErrorOccurs::num_errors = 0; +#ifndef C4_EXCEPTIONS +std::jmp_buf TestErrorOccurs::s_jmp_env_expect_error = {}; +std::string TestErrorOccurs::s_jmp_msg; +#endif } // namespace c4 diff --git a/test/c4/test.hpp b/test/c4/test.hpp index a655dbb1..1780225e 100644 --- a/test/c4/test.hpp +++ b/test/c4/test.hpp @@ -12,6 +12,12 @@ #include #include +#ifdef C4_EXCEPTIONS +#include +#else +#include +#endif + // FIXME - these are just dumb placeholders #define C4_LOGF_ERR(...) fprintf(stderr, __VA_ARGS__) #define C4_LOGF_WARN(...) fprintf(stderr, __VA_ARGS__) @@ -79,7 +85,6 @@ #define CHECK_STREQ(lhs, rhs) CHECK_EQ(c4::to_csubstr(lhs), c4::to_csubstr(rhs)) #define CHECK_FLOAT_EQ(lhs, rhs) CHECK((double)(lhs) == doctest::Approx((double)(rhs))) - namespace c4 { template @@ -89,36 +94,70 @@ inline std::ostream& operator<< (std::ostream& stream, c4::basic_substring s) return stream; } + //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- /** RAII class that tests whether an error occurs inside a scope. */ struct TestErrorOccurs { - TestErrorOccurs(size_t num_expected_errors = 1) - : - expected_errors(num_expected_errors), - tmp_settings(c4::ON_ERROR_CALLBACK, &TestErrorOccurs::error_callback) + static void error_callback(const char* msg, size_t msg_size) { - num_errors = 0; + ++num_errors; + #ifdef C4_EXCEPTIONS + throw std::runtime_error({msg, msg_size}); + #else + s_jmp_msg.assign(msg, msg_size); + std::longjmp(s_jmp_env_expect_error, 37); + #endif } - ~TestErrorOccurs() + + template + TestErrorOccurs(Fn &&fn, const char *expected_msg=nullptr) + : tmp_settings(c4::ON_ERROR_CALLBACK, &TestErrorOccurs::error_callback) { - CHECK_EQ(num_errors, expected_errors); + CHECK_EQ(get_error_callback(), &TestErrorOccurs::error_callback); num_errors = 0; + #ifdef C4_EXCEPTIONS + try + { + fn(); + } + catch (std::runtime_error const& exc) + { + if(expected_msg) + CHECK_EQ(exc.what(), expected_msg); + } + catch (...) + { + CHECK_EQ(1, 0); // fail + } + #else + switch(setjmp(s_jmp_env_expect_error)) + { + case 0: + fn(); + break; + case 37: + // got expected error from call to fn() + if(expected_msg) + CHECK_EQ(s_jmp_msg, expected_msg); + break; + } + #endif + CHECK_EQ(num_errors, 1); } - size_t expected_errors; static size_t num_errors; + #ifndef C4_EXCEPTIONS + static std::jmp_buf s_jmp_env_expect_error; + static std::string s_jmp_msg; + #endif ScopedErrorSettings tmp_settings; - static void error_callback(const char* /*msg*/, size_t /*msg_size*/) - { - ++num_errors; - } }; #define C4_EXPECT_ERROR_OCCURS(...) \ - auto _testerroroccurs##__LINE__ = TestErrorOccurs(__VA_ARGS__) + auto C4_XCAT(_testerroroccurs, __LINE__) = c4::TestErrorOccurs(__VA_ARGS__) #if C4_USE_ASSERT # define C4_EXPECT_ASSERT_TRIGGERS(...) C4_EXPECT_ERROR_OCCURS(__VA_ARGS__) diff --git a/test/test_error.cpp b/test/test_error.cpp index 507f200e..2a5d0499 100644 --- a/test/test_error.cpp +++ b/test/test_error.cpp @@ -9,33 +9,15 @@ #endif C4_SUPPRESS_WARNING_MSVC_WITH_PUSH(4611) // interaction between '_setjmp' and C++ object destruction is non-portable -C4_BEGIN_HIDDEN_NAMESPACE -bool got_an_error = false; -void error_callback(const char *msg, size_t msg_sz) -{ - CHECK_EQ(strncmp(msg, "bla bla", msg_sz), 0); - CHECK_EQ(msg_sz, 7); - got_an_error = true; -} -inline c4::ScopedErrorSettings tmp_err() -{ - got_an_error = false; - return c4::ScopedErrorSettings(c4::ON_ERROR_CALLBACK, error_callback); -} -C4_END_HIDDEN_NAMESPACE - namespace c4 { TEST_CASE("Error.scoped_callback") { auto orig = get_error_callback(); - { - auto tmp = tmp_err(); - CHECK_EQ(get_error_callback() == error_callback, true); + C4_EXPECT_ERROR_OCCURS([&]{ + CHECK_EQ(get_error_callback() != orig, true); C4_ERROR("bla bla"); - CHECK_EQ(got_an_error, true); - } - CHECK_EQ(get_error_callback() == orig, true); + }); } } // namespace c4 @@ -43,13 +25,10 @@ TEST_CASE("Error.scoped_callback") TEST_CASE("Error.outside_of_c4_namespace") { auto orig = c4::get_error_callback(); - { - auto tmp = tmp_err(); - CHECK_EQ(c4::get_error_callback() == error_callback, true); + C4_EXPECT_ERROR_OCCURS([&]{ + CHECK_EQ(c4::get_error_callback() != orig, true); C4_ERROR("bla bla"); - CHECK_EQ(got_an_error, true); - } - CHECK_EQ(c4::get_error_callback() == orig, true); + }); } diff --git a/test/test_error_exception.cpp b/test/test_error_exception.cpp index 0d8812af..f6cf0318 100644 --- a/test/test_error_exception.cpp +++ b/test/test_error_exception.cpp @@ -10,6 +10,7 @@ #include #endif C4_SUPPRESS_WARNING_MSVC_WITH_PUSH(4611) // interaction between '_setjmp' and C++ object destruction is non-portable +C4_SUPPRESS_WARNING_MSVC_WITH_PUSH(4702) // unreachable code C4_BEGIN_HIDDEN_NAMESPACE bool got_an_error = false; diff --git a/test/test_memory_resource.cpp b/test/test_memory_resource.cpp index 4be6f89a..588d0aaf 100644 --- a/test/test_memory_resource.cpp +++ b/test/test_memory_resource.cpp @@ -50,9 +50,10 @@ TEST_CASE("aalloc_impl.error_bad_align") { #if defined(C4_POSIX) #if !defined(C4_ASAN) && !defined(C4_LSAN) && !defined(C4_TSAN) && !defined(C4_UBSAN) - C4_EXPECT_ERROR_OCCURS(1); - auto *mem = detail::aalloc_impl(64, 9); // allocating with a non-power of two value is invalid - CHECK_EQ(mem, nullptr); + C4_EXPECT_ERROR_OCCURS([&]{ + auto *mem = detail::aalloc_impl(64, 9); // allocating with a non-power of two value is invalid + (void)mem; + }); #endif #endif } @@ -62,10 +63,11 @@ TEST_CASE("aalloc_impl.error_out_of_mem") #if defined(C4_POSIX) #if !defined(C4_ASAN) && !defined(C4_LSAN) && !defined(C4_TSAN) && !defined(C4_UBSAN) if(sizeof(size_t) != 8) return; // valgrind complains that size is -1 - C4_EXPECT_ERROR_OCCURS(1); - size_t sz = std::numeric_limits::max() / 2u; - void const* mem = detail::aalloc_impl(sz); - CHECK_EQ(mem, nullptr); + C4_EXPECT_ERROR_OCCURS([&]{ + size_t sz = std::numeric_limits::max() / 2u; + void const* mem = detail::aalloc_impl(sz); + (void)mem; + }); #endif #endif } @@ -158,30 +160,30 @@ TEST_CASE("MemoryResourceLinearArr.reallocate") TEST_CASE("MemoryResourceLinear.error_out_of_mem") { { - C4_EXPECT_ERROR_OCCURS(0); MemoryResourceLinear mr(8); mr.allocate(2); } { - C4_EXPECT_ERROR_OCCURS(2); MemoryResourceLinear mr(8); - mr.allocate(9); + C4_EXPECT_ERROR_OCCURS([&]{ + mr.allocate(9); + }); } } TEST_CASE("MemoryResourceLinearArr.error_out_of_mem") { { - C4_EXPECT_ERROR_OCCURS(0); MemoryResourceLinearArr<8> mr; mr.allocate(2); } { - C4_EXPECT_ERROR_OCCURS(2); MemoryResourceLinearArr<8> mr; - mr.allocate(9); + C4_EXPECT_ERROR_OCCURS([&]{ + mr.allocate(9); + }); } } diff --git a/test/test_szconv.cpp b/test/test_szconv.cpp index d54bf48a..a1c6b2c8 100644 --- a/test/test_szconv.cpp +++ b/test/test_szconv.cpp @@ -107,13 +107,15 @@ test_szconv() #if C4_USE_XASSERT if((uint64_t)omax < (uint64_t)imax) { - C4_EXPECT_ERROR_OCCURS(); - O out = szconv(imax); + C4_EXPECT_ERROR_OCCURS([&]{ + O out = szconv(imax); + }); } else if((uint64_t)omax > (uint64_t)imax) { - C4_EXPECT_ERROR_OCCURS(); - I out = szconv(omax); + C4_EXPECT_ERROR_OCCURS([&]{ + I out = szconv(omax); + }); } #endif }