[gem5-dev] [M] Change in gem5/gem5[develop]: base: Enable non-copiable types in gem5_assert message formatting

2023-02-02 Thread Gabriel B. (Gerrit) via gem5-dev 
Gabriel B. has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/67335?usp=email )


 (

7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
 )Change subject: base: Enable non-copiable types in gem5_assert message  
formatting

..

base: Enable non-copiable types in gem5_assert message formatting

Previous implementation was taking string formatting arguments by value,
which requires copiability or movability. Took the oportunity to scope
the helper functions inside the macro using lambdas.

Change-Id: I2cefc18df1e99b70e60e64588df61eb72a3e5166
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/67335
Tested-by: kokoro 
Maintainer: Bobby Bruce 
Reviewed-by: Bobby Bruce 
---
M src/base/logging.hh
M src/base/logging.test.cc
2 files changed, 32 insertions(+), 21 deletions(-)

Approvals:
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/base/logging.hh b/src/base/logging.hh
index 8949b0c..22fd2a8 100644
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -43,7 +43,6 @@

 #include 
 #include 
-#include 
 #include 

 #include "base/compiler.hh"
@@ -289,24 +288,10 @@
 #define NDEBUG_DEFINED 0
 #endif

-template 
-inline std::string
-_assertMsg(const std::string , Args... args)
-{
-return std::string(": ") + csprintf(format, args...);
-}
-
-inline const char *
-_assertMsg()
-{
-return "";
-}
-
 /**
  * The assert macro will function like a normal assert, but will use panic
  * instead of straight abort(). This allows to perform some cleaning up in
- * ExitLogger::exit() before calling abort(). This macro will not check its
- * condition in fast builds, but it must still be valid code.
+ * ExitLogger::exit() before calling abort().
  *
  * @param cond Condition that is checked; if false -> panic
  * @param ...  Printf-based format string with arguments, extends printout.
@@ -315,11 +300,17 @@
  *
  * @ingroup api_logger
  */
-#define gem5_assert(cond, ...) \
-do { \
-if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) { \
-panic("assert(" #cond ") failed%s", _assertMsg(__VA_ARGS__)); \
-} \
+#define gem5_assert(cond, ...)  \
+do {\
+GEM5_UNLIKELY(NDEBUG_DEFINED || static_cast(cond)) ?  \
+void(0) :   \
+[](const auto&... args) {   \
+auto msg = [&]{ \
+if constexpr (sizeof...(args) == 0) return "";  \
+else return std::string(": ") + csprintf(args...);  \
+};  \
+panic("assert(" #cond ") failed%s", msg()); \
+}(__VA_ARGS__); \
 } while (0)
 /** @} */ // end of api_logger

diff --git a/src/base/logging.test.cc b/src/base/logging.test.cc
index 38cc605..5d10f6e 100644
--- a/src/base/logging.test.cc
+++ b/src/base/logging.test.cc
@@ -553,6 +553,9 @@
 gem5_assert(true, "message\n");
 ASSERT_DEATH(gem5_assert(false, "message\n"), ::testing::HasSubstr(
 "panic: assert(false) failed: message\nMemory Usage:"));
+ASSERT_DEATH(gem5_assert(false, "%s, %s!\n", "Hello", "World"),
+::testing::HasSubstr(
+"panic: assert(false) failed: Hello, World!\nMemory Usage:"));
 gem5_assert(true);
 ASSERT_DEATH(gem5_assert(false), ::testing::HasSubstr(
 "panic: assert(false) failed\nMemory Usage:"));

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/67335?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2cefc18df1e99b70e60e64588df61eb72a3e5166
Gerrit-Change-Number: 67335
Gerrit-PatchSet: 14
Gerrit-Owner: Gabriel B. 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabriel B. 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] [M] Change in gem5/gem5[develop]: base: Enable non-copiable types in gem5_assert message formatting

2023-01-19 Thread Gabriel B. (Gerrit) via gem5-dev 
Gabriel B. has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/67335?usp=email )



Change subject: base: Enable non-copiable types in gem5_assert message  
formatting

..

base: Enable non-copiable types in gem5_assert message formatting

Also:
  - Removed useless inclusion of tuple in logging.h.
  - Turn the bode of gem5_assert into an expression.
  - Added a unit test with formatted assert failure message.

In the previous definition of gem5_assert, the body was a statement (do{}
while(0);) and not an expression. Note that the new definition follows the
same style as GNU assert. Expressions can be used in more situations than
statements.

For instance, expressions can be used with the comma operator. When doing
generic programming, the comma operator helps manipulating parameter
packs.With the previous structure, (gem5_assert(args > 0), ...) could not
be written while perfectly sound and more readable than the current
alternative: assert(((args> 0) && ...)).

Also, (c1 ? a : c2 ?  b : gem5_assert(c3), c) is a usefull expression to
assert completeness of cascaded conditions that cannot be easily achieved
without an expression kind of assertion.

Change-Id: I2cefc18df1e99b70e60e64588df61eb72a3e5166
---
M src/base/logging.hh
M src/base/logging.test.cc
2 files changed, 45 insertions(+), 22 deletions(-)



diff --git a/src/base/logging.hh b/src/base/logging.hh
index 8949b0c..0e81ffe 100644
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -43,7 +43,6 @@

 #include 
 #include 
-#include 
 #include 

 #include "base/compiler.hh"
@@ -289,24 +288,10 @@
 #define NDEBUG_DEFINED 0
 #endif

-template 
-inline std::string
-_assertMsg(const std::string , Args... args)
-{
-return std::string(": ") + csprintf(format, args...);
-}
-
-inline const char *
-_assertMsg()
-{
-return "";
-}
-
 /**
  * The assert macro will function like a normal assert, but will use panic
  * instead of straight abort(). This allows to perform some cleaning up in
- * ExitLogger::exit() before calling abort(). This macro will not check its
- * condition in fast builds, but it must still be valid code.
+ * ExitLogger::exit() before calling abort().
  *
  * @param cond Condition that is checked; if false -> panic
  * @param ...  Printf-based format string with arguments, extends printout.
@@ -315,12 +300,18 @@
  *
  * @ingroup api_logger
  */
-#define gem5_assert(cond, ...) \
-do { \
-if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) { \
-panic("assert(" #cond ") failed%s", _assertMsg(__VA_ARGS__)); \
-} \
-} while (0)
+#define gem5_assert(cond, ...)  \
+(   \
+GEM5_UNLIKELY(NDEBUG_DEFINED || static_cast(cond)) ?  \
+void(0):\
+[](const auto&... args) {   \
+auto msg = [&]{ \
+if constexpr (sizeof...(args) == 0) return "";  \
+else return std::string(": ") + csprintf(args...);  \
+};  \
+panic("assert(" #cond ") failed%s", msg()); \
+}(__VA_ARGS__)  \
+)
 /** @} */ // end of api_logger

 #define chatty_assert(...) \
diff --git a/src/base/logging.test.cc b/src/base/logging.test.cc
index 38cc605..5d10f6e 100644
--- a/src/base/logging.test.cc
+++ b/src/base/logging.test.cc
@@ -553,6 +553,9 @@
 gem5_assert(true, "message\n");
 ASSERT_DEATH(gem5_assert(false, "message\n"), ::testing::HasSubstr(
 "panic: assert(false) failed: message\nMemory Usage:"));
+ASSERT_DEATH(gem5_assert(false, "%s, %s!\n", "Hello", "World"),
+::testing::HasSubstr(
+"panic: assert(false) failed: Hello, World!\nMemory Usage:"));
 gem5_assert(true);
 ASSERT_DEATH(gem5_assert(false), ::testing::HasSubstr(
 "panic: assert(false) failed\nMemory Usage:"));

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/67335?usp=email
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2cefc18df1e99b70e60e64588df61eb72a3e5166
Gerrit-Change-Number: 67335
Gerrit-PatchSet: 1
Gerrit-Owner: Gabriel B. 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org