Merge pull request #2146 from elBoberido/iox-1032-refactor-iox-require

iox-#1032 Refactor 'IOX_REQUIRE'

doc/design/error_reporting.md

@@ -165,7 +165,7 @@ the case that it does not hold
 ```cpp
 int x;
 // ...
-IOX_REQUIRE(x>=0, Code::OutOfBounds);
+IOX_REQUIRE(x>=0, "required condition violation message");
 ```
 
 The condition is required to hold and this requirement is always checked.

iceoryx_hoofs/reporting/include/iox/assertions.hpp

@@ -44,19 +44,29 @@
         iox::er::forwardPanic(CURRENT_SOURCE_LOCATION, message);                                                       \
     } while (false)
 
-/// @brief report fatal error if expr evaluates to false
-/// @note for conditions that may actually happen during correct use
-/// @param expr boolean expression that must hold
-/// @param error error object (or code)
-#define IOX_REQUIRE(expr, error) IOX_REPORT_FATAL_IF(!(expr), error)
-
 //************************************************************************************************
 //* For documentation of intent, defensive programming and debugging
 //*
 //* There are no error codes/errors required here on purpose, as it would make the use cumbersome.
 //* Instead a special internal error type is used.
 //************************************************************************************************
 
+/// @brief report fatal required condition violation if expr evaluates to false
+/// @note for conditions that may actually happen during correct use
+/// @param expr boolean expression that must hold
+/// @param message message to be forwarded in case of violation
+#define IOX_REQUIRE(expr, message)                                                                                     \
+    do                                                                                                                 \
+    {                                                                                                                  \
+        if (!(expr))                                                                                                   \
+        {                                                                                                              \
+            iox::er::forwardFatalError(iox::er::Violation::createRequiredConditionViolation(),                         \
+                                       iox::er::REQUIRED_CONDITION_VIOLATION,                                          \
+                                       CURRENT_SOURCE_LOCATION,                                                        \
+                                       message); /* @todo iox-#1032 add strigified 'expr' as '#expr' */                \
+        }                                                                                                              \
+    } while (false)
+
 /// @brief if enabled: report fatal precondition violation if expr evaluates to false
 /// @param expr boolean expression that must hold upon entry of the function it appears in
 /// @param message message to be forwarded in case of violation

iceoryx_hoofs/reporting/include/iox/error_reporting/custom/default/error_reporting_impl.hpp

@@ -67,6 +67,8 @@ template <class Message>
     panic();
 }
 
+/// @todo iox-#1032 add a 'StringifiedCondition' template parameter
+
 // Report any error, general version.
 template <class Kind, class Error>
 inline void report(const SourceLocation& location, Kind, const Error& error)
@@ -103,46 +105,67 @@ inline void report(const SourceLocation& location, iox::er::FatalKind kind, cons
     h.onReportError(ErrorDescriptor(location, code, module));
 }
 
-template <class Error>
-inline void report(const SourceLocation& location, iox::er::PreconditionViolationKind kind, const Error& error)
+namespace detail
+{
+enum class NoMessage
+{
+};
+
+template <class Kind, class Error, class Message>
+inline void report(const SourceLocation& location, Kind kind, const Error& error, Message&& msg)
 {
     auto code = toCode(error);
     auto module = toModule(error);
-    IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name);
+    if constexpr (std::is_same<NoMessage, Message>::value)
+    {
+        IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name);
+    }
+    else
+    {
+        IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name << " " << std::forward<Message>(msg));
+    }
     auto& h = ErrorHandler::get();
     h.onReportViolation(ErrorDescriptor(location, code, module));
 }
+} // namespace detail
+
+template <class Error>
+inline void report(const SourceLocation& location, iox::er::RequiredConditionViolationKind kind, const Error& error)
+{
+    detail::report(location, kind, error, detail::NoMessage{});
+}
+
+template <class Error>
+inline void report(const SourceLocation& location, iox::er::PreconditionViolationKind kind, const Error& error)
+{
+    detail::report(location, kind, error, detail::NoMessage{});
+}
 
 template <class Error>
 inline void report(const SourceLocation& location, iox::er::AssumptionViolationKind kind, const Error& error)
 {
-    auto code = toCode(error);
-    auto module = toModule(error);
-    IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name);
-    auto& h = ErrorHandler::get();
-    h.onReportViolation(ErrorDescriptor(location, code, module));
+    detail::report(location, kind, error, detail::NoMessage{});
+}
+
+template <class Error, class Message>
+inline void
+report(const SourceLocation& location, iox::er::RequiredConditionViolationKind kind, const Error& error, Message&& msg)
+{
+    detail::report(location, kind, error, std::forward<Message>(msg));
 }
 
 template <class Error, class Message>
 inline void
 report(const SourceLocation& location, iox::er::PreconditionViolationKind kind, const Error& error, Message&& msg)
 {
-    auto code = toCode(error);
-    auto module = toModule(error);
-    IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name << " " << std::forward<Message>(msg));
-    auto& h = ErrorHandler::get();
-    h.onReportViolation(ErrorDescriptor(location, code, module));
+    detail::report(location, kind, error, std::forward<Message>(msg));
 }
 
 template <class Error, class Message>
 inline void
 report(const SourceLocation& location, iox::er::AssumptionViolationKind kind, const Error& error, Message&& msg)
 {
-    auto code = toCode(error);
-    auto module = toModule(error);
-    IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name << " " << std::forward<Message>(msg));
-    auto& h = ErrorHandler::get();
-    h.onReportViolation(ErrorDescriptor(location, code, module));
+    detail::report(location, kind, error, std::forward<Message>(msg));
 }
 
 } // namespace er

iceoryx_hoofs/reporting/include/iox/error_reporting/error_kind.hpp

@@ -37,6 +37,11 @@ struct FatalKind
     static constexpr char const* name = "Fatal Error";
 };
 
+struct RequiredConditionViolationKind
+{
+    static constexpr char const* name = "Required Condition Violation";
+};
+
 struct PreconditionViolationKind
 {
     static constexpr char const* name = "Precondition Violation";
@@ -62,6 +67,11 @@ struct IsFatal<FatalKind> : public std::true_type
 {
 };
 
+template <>
+struct IsFatal<RequiredConditionViolationKind> : public std::true_type
+{
+};
+
 template <>
 struct IsFatal<PreconditionViolationKind> : public std::true_type
 {
@@ -86,6 +96,12 @@ bool constexpr isFatal<FatalKind>(FatalKind)
     return IsFatal<FatalKind>::value;
 }
 
+template <>
+bool constexpr isFatal<RequiredConditionViolationKind>(RequiredConditionViolationKind)
+{
+    return IsFatal<RequiredConditionViolationKind>::value;
+}
+
 template <>
 bool constexpr isFatal<PreconditionViolationKind>(PreconditionViolationKind)
 {
@@ -101,6 +117,9 @@ bool constexpr isFatal<AssumptionViolationKind>(AssumptionViolationKind)
 // indicates serious condition, unable to continue
 constexpr FatalKind FATAL;
 
+// indicates a bug (contract breach by caller)
+constexpr RequiredConditionViolationKind REQUIRED_CONDITION_VIOLATION;
+
 // indicates a bug (contract breach by caller)
 constexpr PreconditionViolationKind PRECONDITION_VIOLATION;
 

iceoryx_hoofs/reporting/include/iox/error_reporting/errors.hpp

@@ -70,6 +70,11 @@ class Violation
         return !(*this == rhs);
     }
 
+    static Violation createRequiredConditionViolation()
+    {
+        return Violation(ErrorCode(ErrorCode::REQUIRED_CONDITION_VIOLATION));
+    }
+
     static Violation createPreconditionViolation()
     {
         return Violation(ErrorCode(ErrorCode::PRECONDITION_VIOLATION));

iceoryx_hoofs/reporting/include/iox/error_reporting/macros.hpp

@@ -61,7 +61,10 @@
     {                                                                                                                  \
         if (expr)                                                                                                      \
         {                                                                                                              \
-            iox::er::forwardNonFatalError(iox::er::toError(error), kind, CURRENT_SOURCE_LOCATION);                     \
+            iox::er::forwardNonFatalError(                                                                             \
+                iox::er::toError(error),                                                                               \
+                kind,                                                                                                  \
+                CURRENT_SOURCE_LOCATION); /* @todo iox-#1032 add strigified 'expr' as '#expr' */                       \
         }                                                                                                              \
     } while (false)
 

iceoryx_hoofs/reporting/include/iox/error_reporting/types.hpp

@@ -33,8 +33,9 @@ struct ErrorCode
 
     type value;
 
-    static constexpr type ASSUMPTION_VIOLATION{0};
-    static constexpr type PRECONDITION_VIOLATION{1};
+    static constexpr type REQUIRED_CONDITION_VIOLATION{0};
+    static constexpr type ASSUMPTION_VIOLATION{1};
+    static constexpr type PRECONDITION_VIOLATION{2};
 
     constexpr explicit ErrorCode(uint32_t value)
         : value(value)

iceoryx_hoofs/test/moduletests/error_reporting/test_error_reporting_macros.cpp

@@ -128,25 +128,25 @@ TEST_F(ErrorReportingMacroApi_test, reportConditionalNoError)
     IOX_TESTING_EXPECT_OK();
 }
 
-TEST_F(ErrorReportingMacroApi_test, requireConditionSatisfied)
+TEST_F(ErrorReportingMacroApi_test, checkRequireConditionSatisfied)
 {
     ::testing::Test::RecordProperty("TEST_ID", "3c684878-20f8-426f-bb8b-7576b567d04f");
-    auto f = []() { IOX_REQUIRE(true, MyCodeA::OutOfBounds); };
+    auto f = []() { IOX_REQUIRE(true, ""); };
 
     runInTestThread(f);
 
     IOX_TESTING_EXPECT_OK();
 }
 
-TEST_F(ErrorReportingMacroApi_test, requireConditionNotSatisfied)
+TEST_F(ErrorReportingMacroApi_test, checkRequireConditionViolate)
 {
     ::testing::Test::RecordProperty("TEST_ID", "fb62d315-8854-401b-82af-6161ae45a34e");
-    auto f = []() { IOX_REQUIRE(false, MyCodeA::OutOfBounds); };
+    auto f = []() { IOX_REQUIRE(false, ""); };
 
     runInTestThread(f);
 
     IOX_TESTING_EXPECT_PANIC();
-    IOX_TESTING_EXPECT_ERROR(MyCodeA::OutOfBounds);
+    IOX_TESTING_EXPECT_REQUIRED_CONDITION_VIOLATION();
 }
 
 TEST_F(ErrorReportingMacroApi_test, checkPreconditionSatisfied)

iceoryx_hoofs/testing/error_reporting/testing_support.cpp

@@ -31,6 +31,12 @@ bool hasError()
     return ErrorHandler::instance().hasError();
 }
 
+bool hasRequiredConditionViolation()
+{
+    auto code = iox::er::ErrorCode{iox::er::ErrorCode::REQUIRED_CONDITION_VIOLATION};
+    return ErrorHandler::instance().hasViolation(code);
+}
+
 bool hasPreconditionViolation()
 {
     auto code = iox::er::ErrorCode{iox::er::ErrorCode::PRECONDITION_VIOLATION};
@@ -45,7 +51,7 @@ bool hasAssumptionViolation()
 
 bool hasViolation()
 {
-    return hasPreconditionViolation() || hasAssumptionViolation();
+    return hasRequiredConditionViolation() || hasPreconditionViolation() || hasAssumptionViolation();
 }
 
 bool isInNormalState()

iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/error_reporting/testing_support.hpp

@@ -50,6 +50,9 @@ bool hasPanicked();
 /// @brief indicates whether the test error handler registered any error
 bool hasError();
 
+/// @brief indicates whether the test error handler registered a required condition violation
+bool hasRequiredConditionViolation();
+
 /// @brief indicates whether the test error handler registered a precondition violation
 bool hasPreconditionViolation();
 
@@ -120,11 +123,11 @@ inline void runInTestThread(Function&& testFunction, Args&&... args)
 
 #define IOX_TESTING_ASSERT_NO_ERROR() ASSERT_FALSE(iox::testing::hasError())
 
-#define IOX_TESTING_ASSERT_VIOLATION()                                                                                 \
-    ASSERT_TRUE(iox::testing::hasPreconditionViolation() || iox::testing::hasAssumptionViolation())
+#define IOX_TESTING_ASSERT_VIOLATION() ASSERT_TRUE(iox::testing::hasViolation())
+
+#define IOX_TESTING_ASSERT_NO_VIOLATION() ASSERT_FALSE(iox::testing::hasViolation())
 
-#define IOX_TESTING_ASSERT_NO_VIOLATION()                                                                              \
-    ASSERT_FALSE(iox::testing::hasPreconditionViolation() || iox::testing::hasAssumptionViolation())
+#define IOX_TESTING_ASSERT_REQUIRED_CONDITION_VIOLATION() ASSERT_TRUE(iox::testing::hasRequiredConditionViolation())
 
 #define IOX_TESTING_ASSERT_PRECONDITION_VIOLATION() ASSERT_TRUE(iox::testing::hasPreconditionViolation())
 
@@ -142,11 +145,11 @@ inline void runInTestThread(Function&& testFunction, Args&&... args)
 
 #define IOX_TESTING_EXPECT_NO_ERROR() EXPECT_FALSE(iox::testing::hasError())
 
-#define IOX_TESTING_EXPECT_VIOLATION()                                                                                 \
-    EXPECT_TRUE(iox::testing::hasPreconditionViolation() || iox::testing::hasAssumptionViolation())
+#define IOX_TESTING_EXPECT_VIOLATION() EXPECT_TRUE(iox::testing::hasViolation())
+
+#define IOX_TESTING_EXPECT_NO_VIOLATION() EXPECT_FALSE(iox::testing::hasViolation())
 
-#define IOX_TESTING_EXPECT_NO_VIOLATION()                                                                              \
-    EXPECT_FALSE(iox::testing::hasPreconditionViolation() || iox::testing::hasAssumptionViolation())
+#define IOX_TESTING_EXPECT_REQUIRED_CONDITION_VIOLATION() EXPECT_TRUE(iox::testing::hasRequiredConditionViolation())
 
 #define IOX_TESTING_EXPECT_PRECONDITION_VIOLATION() EXPECT_TRUE(iox::testing::hasPreconditionViolation())