From fbed7c34bb5e3bc6ac42b397d232886591f79dfe Mon Sep 17 00:00:00 2001
From: Stephane Del Pino <stephane.delpino44@gmail.com>
Date: Mon, 1 Jun 2020 01:22:28 +0200
Subject: [PATCH] Improve Assert macro

- do not use anymore gcc extension for __VA_ARGS__

- check that parameters are always correct (string literal could be
unchecked under NDEBUG)

- when using NDEBUG, assert exception cannot be evaluated. Previously
result was not used but non trivial assertions such as
  `container.find(key) != container.end()`
could be evaluated. More specifically functions with potential
boundary effects.

- also, this allows to compile the code with the `-pedantic` flag.
---
 CMakeLists.txt            |  2 +-
 src/utils/PugsAssert.hpp  | 96 ++++++++++++++++++++++++++++-----------
 src/utils/PugsTraits.hpp  | 28 ++++++++++++
 tests/test_PugsAssert.cpp |  9 ++--
 4 files changed, 102 insertions(+), 33 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e03015bec..168fe3a82 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -85,7 +85,7 @@ set(GNU_CXX_MIN_VERSION "7.0.0")
 set(CLANG_CXX_MIN_VERSION "5.0.0")
 
 # Pugs default compiler flags
-set(PUGS_CXX_FLAGS "${PUGS_CXX_FLAGS} -Wall -Wextra")
+set(PUGS_CXX_FLAGS "${PUGS_CXX_FLAGS} -Wall -Wextra -pedantic")
 
 if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
   if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "${GNU_CXX_MIN_VERSION}")
diff --git a/src/utils/PugsAssert.hpp b/src/utils/PugsAssert.hpp
index 810cc26db..def5604de 100644
--- a/src/utils/PugsAssert.hpp
+++ b/src/utils/PugsAssert.hpp
@@ -2,6 +2,7 @@
 #define PUGS_ASSERT_HPP
 
 #include <utils/PugsMacros.hpp>
+#include <utils/PugsTraits.hpp>
 
 #include <iostream>
 #include <rang.hpp>
@@ -12,7 +13,7 @@ class AssertError
 {
  private:
   const std::string m_file;
-  const int m_line;
+  const size_t m_line;
   const std::string m_function;
   const std::string m_test;
   const std::string m_message;
@@ -22,52 +23,95 @@ class AssertError
   operator<<(std::ostream& os, const AssertError& assert_error)
   {
     os << '\n'
-       << rang::style::bold << "---------- Assertion error -----------\n"
+       << "---------- Assertion error -----------\n"
        << " at " << assert_error.m_file << ':' << assert_error.m_line << '\n'
        << " in " << assert_error.m_function << '\n'
-       << " assertion (" << rang::fgB::red << assert_error.m_test << rang::fg::reset << ") failed!\n";
+       << " assertion (" << assert_error.m_test << ") failed!\n";
     if (not assert_error.m_message.empty()) {
-      os << ' ' << rang::fgB::yellow << assert_error.m_message << rang::fg::reset << '\n';
+      os << ' ' << assert_error.m_message << '\n';
     }
-    os << "--------------------------------------" << rang::style::reset << '\n';
+    os << "--------------------------------------" << '\n';
 
     return os;
   }
 
-  AssertError(const AssertError&) = default;
-  AssertError(const std::string& file,
-              int line,
+  template <typename... Args>
+  AssertError(const std::string& filename,
+              size_t line,
               const std::string& function,
-              const std::string& test,
-              const std::string& message = "")
-    : m_file(file), m_line(line), m_function(function), m_test(test), m_message(message)
+              const std::tuple<Args...>& tuple_args,
+              const std::string_view args_string)
+    : m_file{filename},
+      m_line{line},
+      m_function{function},
+      m_test{[&] {
+        if constexpr (sizeof...(Args) == 1) {
+          return std::string(args_string);
+        } else {
+          std::string_view message = std::get<1>(tuple_args);
+          std::string test_str{args_string, 0, args_string.size() - message.size() - 2};
+          while (test_str[test_str.size() - 1] == ' ' or test_str[test_str.size() - 1] == ',') {
+            test_str.resize(test_str.size() - 1);
+          }
+          return test_str;
+        }
+      }()},
+      m_message{[&] {
+        if constexpr (sizeof...(Args) == 1) {
+          return std::string{};
+        } else {
+          return std::get<1>(tuple_args);
+        }
+      }()}
   {
     ;
   }
+};
+
+template <typename... Args>
+struct AssertChecker;
+
+template <typename... Args>
+struct AssertChecker<std::tuple<Args...> >
+{
+  void static check_args_type()
+  {
+    constexpr size_t size = sizeof...(Args);
+    static_assert(size >= 1 and size <= 2, "Assert requires 1 or 2 parameters");
 
-  ~AssertError() = default;
+    using ArgsTuple = std::tuple<Args...>;
+
+    using assertion_t = std::tuple_element_t<0, ArgsTuple>;
+    static_assert(std::is_integral_v<assertion_t> or std::is_pointer_v<assertion_t> or is_std_ptr_v<assertion_t>);
+
+    if constexpr (size == 2) {
+      using message_t = std::decay_t<std::remove_const_t<std::tuple_element_t<1, ArgsTuple> > >;
+
+      static_assert(std::is_same_v<message_t, const char*>, "optional second argument must be a string literal");
+    }
+  }
 };
 
 #ifdef NDEBUG
 
-// Useless test is there to check syntax even in optimized mode. Costs nothing.
-#define Assert(assertion, ...)                                            \
-  if (not static_cast<bool>(assertion)) {                                 \
-    using vargs_t = decltype(std::make_tuple(__VA_ARGS__));               \
-    static_assert(std::tuple_size_v<vargs_t> <= 1, "too many arguments"); \
+#define Assert(...)                                           \
+  {                                                           \
+    using TupleArgs = decltype(std::make_tuple(__VA_ARGS__)); \
+    AssertChecker<TupleArgs>::check_args_type();              \
   }
 
 #else   // NDEBUG
 
-#define Assert(assertion, ...)                                                              \
-  if (not static_cast<bool>(assertion)) {                                                   \
-    using vargs_t = decltype(std::make_tuple(__VA_ARGS__));                                 \
-    static_assert(std::tuple_size_v<vargs_t> <= 1, "too many arguments");                   \
-    if constexpr (std::tuple_size_v<vargs_t> == 0) {                                        \
-      throw AssertError(__FILE__, __LINE__, __PRETTY_FUNCTION__, #assertion);               \
-    } else {                                                                                \
-      throw AssertError(__FILE__, __LINE__, __PRETTY_FUNCTION__, #assertion, #__VA_ARGS__); \
-    }                                                                                       \
+#define Assert(...)                                                                   \
+  {                                                                                   \
+    using TupleArgs = decltype(std::make_tuple(__VA_ARGS__));                         \
+    AssertChecker<TupleArgs>::check_args_type();                                      \
+    constexpr int tuple_size = std::tuple_size_v<TupleArgs>;                          \
+    static_assert(tuple_size >= 1 and tuple_size <= 2);                               \
+    auto args = std::forward_as_tuple(__VA_ARGS__);                                   \
+    if (not static_cast<bool>(std::get<0>(args))) {                                   \
+      throw AssertError(__FILE__, __LINE__, __PRETTY_FUNCTION__, args, #__VA_ARGS__); \
+    }                                                                                 \
   }
 
 #endif   // NDEBUG
diff --git a/src/utils/PugsTraits.hpp b/src/utils/PugsTraits.hpp
index c45457812..cb934a2ab 100644
--- a/src/utils/PugsTraits.hpp
+++ b/src/utils/PugsTraits.hpp
@@ -39,6 +39,34 @@ inline constexpr bool is_shared_ptr_v = false;
 template <typename T>
 inline constexpr bool is_shared_ptr_v<std::shared_ptr<T>> = true;
 
+// Traits is_shared_ptr
+
+template <typename T>
+inline constexpr bool is_unique_ptr_v = false;
+
+template <typename T>
+inline constexpr bool is_unique_ptr_v<std::unique_ptr<T>> = true;
+
+// Traits is_weak_ptr
+
+template <typename T>
+inline constexpr bool is_weak_ptr_v = false;
+
+template <typename T>
+inline constexpr bool is_weak_ptr_v<std::weak_ptr<T>> = true;
+
+// Traits is_std_ptr
+
+template <typename T>
+inline constexpr bool is_std_ptr_v = false;
+
+template <typename T>
+inline constexpr bool is_std_ptr_v<std::shared_ptr<T>> = true;
+template <typename T>
+inline constexpr bool is_std_ptr_v<std::unique_ptr<T>> = true;
+template <typename T>
+inline constexpr bool is_std_ptr_v<std::weak_ptr<T>> = true;
+
 // Traits is_vector
 
 template <typename T>
diff --git a/tests/test_PugsAssert.cpp b/tests/test_PugsAssert.cpp
index b28ee8814..a7fa737f8 100644
--- a/tests/test_PugsAssert.cpp
+++ b/tests/test_PugsAssert.cpp
@@ -11,11 +11,10 @@ TEST_CASE("PugsAssert", "[utils]")
   SECTION("checking for assert error")
   {
     const std::string filename = "filename";
-    const int line             = 10;
+    const size_t line          = 10;
     const std::string function = "function";
-    const std::string test     = "test";
 
-    AssertError assert_error(filename, line, function, test);
+    AssertError assert_error(filename, line, function, std::make_tuple("test"), "test");
 
     REQUIRE(Catch::Detail::stringify(assert_error) == "\n---------- Assertion error -----------\n at filename:10\n in "
                                                       "function\n assertion (test) "
@@ -27,10 +26,8 @@ TEST_CASE("PugsAssert", "[utils]")
     const std::string filename = "filename";
     const int line             = 10;
     const std::string function = "function";
-    const std::string test     = "test";
-    const std::string message  = "message";
 
-    AssertError assert_error(filename, line, function, test, message);
+    AssertError assert_error(filename, line, function, std::make_tuple(false, "message"), "test, message  ");
 
     REQUIRE(Catch::Detail::stringify(assert_error) == "\n---------- Assertion error -----------\n at filename:10\n in "
                                                       "function\n assertion (test) failed!\n "
-- 
GitLab