Merge pull request #137 from biojppm/error_noreturn

Error noreturn
biojppm · Jul 6, 2024 · edd2649 · edd2649
2 parents aa7cf86 + f739367
commit edd2649
Show file tree

Hide file tree

Showing 13 changed files with 111 additions and 77 deletions.
diff --git 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 `<charconv>`.
-- Add `bool from_chars(csubstr s, fmt::overflow_checked_<T> *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_<T> *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
@@ -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
@@ -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,44 +104,44 @@ 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();
 }
 
 //-----------------------------------------------------------------------------
 
 void handle_warning(srcloc where, const char *fmt, ...)
 {
     va_list args;
-    char buf[1024]; //sstream<c4::string> 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
@@ -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
@@ -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
@@ -53,7 +53,7 @@ szconv(SizeIn sz) noexcept
 template<class SizeOut, class SizeIn>
 C4_ALWAYS_INLINE
 typename std::enable_if<is_narrower_size<SizeOut, SizeIn>::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<SizeOut>::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
@@ -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
@@ -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
@@ -12,6 +12,12 @@
 #include <cstdio>
 #include <iostream>
 
+#ifdef C4_EXCEPTIONS
+#include <stdexcept>
+#else
+#include <csetjmp>
+#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<class C>
@@ -89,36 +94,70 @@ inline std::ostream& operator<< (std::ostream& stream, c4::basic_substring<C> 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<class Fn>
+    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
@@ -9,47 +9,26 @@
 #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
 
 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
@@ -10,6 +10,7 @@
 #include <csetjmp>
 #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
@@ -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<size_t>::max() / 2u;
-    void const* mem = detail::aalloc_impl(sz);
-    CHECK_EQ(mem, nullptr);
+    C4_EXPECT_ERROR_OCCURS([&]{
+        size_t sz = std::numeric_limits<size_t>::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);
+        });
     }
 }