From 6af003771dc9326a3226829f54b7498ff49f2c2b Mon Sep 17 00:00:00 2001 From: Eidolon Date: Wed, 28 Dec 2022 22:01:47 -0600 Subject: [PATCH] Add SRB2_ASSERT, srb2::NotNull Add SRB2_ASSERT, superceding I_Assert This assertion macro always expands to a call of srb2::do_assert, which is overloaded with two templates: one which applies if the provided Level is less than or equal to the SRB2_ASSERTION_LEVEL, and one which is a no-op. When optimizations are enabled, this will verifiably remove the evaluation of the expression in all cases, instead of evaluating the expression and doing nothing with it. Add srb2::NotNull wrapper utility This is meant to be used in places where pointers are used as parameters. It can be used with any pointer-like type, not just raw pointers. During construction of NotNull, the pointer will be asserted not-null in debug and paranoia builds, and in release optimizations with no assertions, the code decays gracefully to standard pointer-passing. --- src/cxxutil.hpp | 117 ++++++++++++++++++++++++++++++++++++++- src/doomdef.h | 1 + src/tests/CMakeLists.txt | 2 + src/tests/boolcompat.cpp | 1 + src/tests/notnull.cpp | 31 +++++++++++ src/tests/testbase.hpp | 6 ++ 6 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 src/tests/notnull.cpp create mode 100644 src/tests/testbase.hpp diff --git a/src/cxxutil.hpp b/src/cxxutil.hpp index befe9c20b..74349a0ff 100644 --- a/src/cxxutil.hpp +++ b/src/cxxutil.hpp @@ -1,9 +1,12 @@ #ifndef __SRB2_CXXUTIL_HPP__ #define __SRB2_CXXUTIL_HPP__ +#include #include #include +#include "doomdef.h" + namespace srb2 { template @@ -19,7 +22,8 @@ public: void operator=(Finally&& from) = delete; ~Finally() noexcept { - f_(); + if (call_) + f_(); } private: @@ -32,6 +36,117 @@ Finally> finally(F&& f) noexcept { return Finally {std::forward(f)}; } +struct SourceLocation { + const char* file_name; + unsigned int line_number; +}; + +#define SRB2_SOURCE_LOCATION \ + srb2::SourceLocation { __FILE__, __LINE__ } + +#ifndef SRB2_ASSERT_HANDLER +#define SRB2_ASSERT_HANDLER srb2::IErrorAssertHandler +#endif + +#if !defined(NDEBUG) || defined(PARANOIA) +// An assertion level of 2 will activate all invocations of the SRB2_ASSERT macro +#define SRB2_ASSERTION_LEVEL 2 +#else +// The minimum assertion level is 1 +#define SRB2_ASSERTION_LEVEL 1 +#endif + +/// Assert a precondition expression in debug builds. +#define SRB2_ASSERT(expr) srb2::do_assert<2, SRB2_ASSERT_HANDLER>([&] { return (expr); }, SRB2_SOURCE_LOCATION, #expr) + +class IErrorAssertHandler { +public: + static void handle(const SourceLocation& source_location, const char* expression) { + I_Error("Assertion failed at %s:%u: %s != true", + source_location.file_name, + source_location.line_number, + expression); + } +}; + +class NoOpAssertHandler { +public: + static void handle(const SourceLocation& source_location, const char* expression) {} +}; + +/// @brief Assert a precondition expression, aborting the application if it fails. +/// @tparam Expr +/// @tparam Level the level of this assertion; if it is less than or equal to SRB2_ASSERTION_LEVEL, this overload will +/// activate. +/// @param expr a callable which returns a bool +/// @param source_location a struct containing the source location of the assertion, e.g. SRB2_SOURCE_LOCATION +/// @param expression the expression evaluated in the expression callable +/// @param message an optional message to display for the assertion +template +std::enable_if_t<(Level <= SRB2_ASSERTION_LEVEL), void> +do_assert(const Expr& expr, const SourceLocation& source_location, const char* expression = "") noexcept { + static_assert(Level > 0, "level of an assertion must not be 0"); + if (!expr()) { + Handler::handle(source_location, expression); + std::abort(); + } } +template +std::enable_if_t<(Level > SRB2_ASSERTION_LEVEL), void> +do_assert(const Expr&, const SourceLocation&, const char* = "") noexcept { +} + +template +class NotNull final { + T ptr_; + +public: + static_assert(std::is_convertible_v() != nullptr), bool>, + "T is not comparable with nullptr_t"); + + /// @brief Move-construct from the pointer value U, asserting that it is not null. Allows construction of a + /// NotNull from any compatible pointer U, for example with polymorphic classes. + template >> + constexpr NotNull(U&& rhs) : ptr_(std::forward(rhs)) { + SRB2_ASSERT(ptr_ != nullptr); + } + + /// @brief Wrap the pointer type T, asserting that the pointer is not null. + template >> + constexpr NotNull(T rhs) : ptr_(std::move(rhs)) { + SRB2_ASSERT(ptr_ != nullptr); + } + + /// @brief Copy construction from NotNull of convertible type U. Only if the incoming pointer is NotNull already. + template >> + constexpr NotNull(const NotNull& rhs) : NotNull(rhs.get()) { + // Value is guaranteed to be not null by construction; no assertion necessary + } + + NotNull(const NotNull& rhs) = default; + NotNull& operator=(const NotNull& rhs) = default; + + /// @brief Get the stored pointer. + constexpr T get() const { return ptr_; } + + /// @brief Convert to T (the pointer type). + constexpr operator T() const { return get(); } + + /// @brief Arrow-dereference to *T (the actual value pointed to). + constexpr decltype(auto) operator->() const { return get(); } + + /// @brief Dereference to *T (the actual value pointed to). + constexpr decltype(auto) operator*() const { return *get(); } + + // It is not allowed to construct NotNull with nullptr regardless of T. + NotNull(std::nullptr_t) = delete; + NotNull& operator=(std::nullptr_t) = delete; +}; + +template +NotNull(T) -> NotNull; + +} // namespace srb2 + #endif // __SRB2_CXXUTIL_HPP__ diff --git a/src/doomdef.h b/src/doomdef.h index 31892fb28..09a5efa4a 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -645,6 +645,7 @@ UINT32 quickncasehash (const char *p, size_t n) #endif // An assert-type mechanism. +// NOTE: USE SRB2_ASSERT FOR C++ CODE INSTEAD #ifdef PARANOIA #define I_Assert(e) ((e) ? (void)0 : I_Error("assert failed: %s, file %s, line %d", #e, __FILE__, __LINE__)) #else diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 28c4ce492..59c64ae00 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -1,3 +1,5 @@ target_sources(srb2tests PRIVATE boolcompat.cpp + notnull.cpp + testbase.hpp ) diff --git a/src/tests/boolcompat.cpp b/src/tests/boolcompat.cpp index fee40cd36..72e2ccba7 100644 --- a/src/tests/boolcompat.cpp +++ b/src/tests/boolcompat.cpp @@ -1,3 +1,4 @@ +#include "testbase.hpp" #include #include "../doomtype.h" diff --git a/src/tests/notnull.cpp b/src/tests/notnull.cpp new file mode 100644 index 000000000..282ef9e57 --- /dev/null +++ b/src/tests/notnull.cpp @@ -0,0 +1,31 @@ +#include "testbase.hpp" +#include + +#include "../cxxutil.hpp" + +namespace { +class A { +public: + virtual bool foo() = 0; +}; +class B : public A { +public: + virtual bool foo() override final { return true; }; +}; +} // namespace + +TEST_CASE("NotNull is constructible from int*") { + int a = 0; + REQUIRE(srb2::NotNull(static_cast(&a))); +} + +TEST_CASE("NotNull is constructible from B* where B inherits from A") { + B b; + REQUIRE(srb2::NotNull(static_cast(&b))); +} + +TEST_CASE("NotNull dereferences to B& to call foo") { + B b; + srb2::NotNull a {static_cast(&b)}; + REQUIRE(a->foo()); +} diff --git a/src/tests/testbase.hpp b/src/tests/testbase.hpp new file mode 100644 index 000000000..39ce223a0 --- /dev/null +++ b/src/tests/testbase.hpp @@ -0,0 +1,6 @@ +#ifndef __SRB2_TESTS_TESTBASE_HPP__ +#define __SRB2_TESTS_TESTBASE_HPP__ + +#define SRB2_ASSERT_HANDLER srb2::NoOpAssertHandler + +#endif // __SRB2_TESTS_TESTBASE_HPP__