Introduce CELLO_ERROR, CELLO_REQUIRE, CELLO_WARN

doc/source/devel/coding-guidelines.rst

@@ -455,23 +455,33 @@ We recommend some of the following coding practices:
        This generally leads to more explicit code.
        Furthermore, it lets the compiler identify cases where the order of an integer argument and a scoped enum argument are accidentally permuted.
 
-* **ERROR REPORTING:** We don't use C++ exceptions in the codebase. In general, when an error arises, we generally abort the program with an informative error message. You can signal that an error occured by using the ``ERROR`` family of macros or you can use the ``ASSERT`` family of macros to have the program conditionally abort if some condition is not satisfied.
+* **ERROR REPORTING:** We don't use C++ exceptions in the codebase. In general, when an error arises, we generally abort the program with an informative error message. You can signal that an error occured by using the :c:macro:`!CELLO_ERROR` function-like macro. Alternatively you can use the :c:macro:`!CELLO_ABORT` function-like macro to conditionally abort if some condition is not satisfied.
 
-  * When you are implementing new functionality, you are encouraged to liberally use the :c:macro:`!ERROR` and :c:macro:`!ASSERT` families of macros to ensure that Enzo-E loudly fails and aborts when the functionality is used in unexpected ways.
+  * When you are implementing new functionality, you are encouraged to liberally use the :c:macro:`!CELLO_ERROR` and :c:macro:`!CELLO_REQUIRE` macros to ensure that Enzo-E loudly fails and aborts when the functionality is used in unexpected ways.
     When running a really expensive simulation, a user should generally prefer that a simulation loudly fails.
     The extreme alternative case is for the simulation to run to completion while silently having problems, which likely invalidates the results (and these problems may not be detected until MUCH later).
     Furthermore, it's easy enough for a user to comment out an error message that they wish to ignore.
 
-  * We briefly describe the arguments of :c:macro:`!ERROR`, :c:macro:`!ERROR1`, :c:macro:`!ERROR2`, ..., :c:macro:`!ERROR8`, :c:macro:`!ASSERT`, :c:macro:`!ASSERT1`, :c:macro:`!ASSERT2`, ..., :c:macro:`!ASSERT8` down below:
+  * in more detail, you can treat the macros as though they are functions with the following signatures:
 
-    * The first argument is always the name of the function where the macro is being invoked (to assist debugging in the future).
+    .. code-block:: c++
 
-    * The second argument is a c-string that provides the error message.
-      Printf formatting specifiers can be used within the error message, but the number of specifiers must match the integer at the end of the macro (e.g. :c:macro:`!ERROR2` or :c:macro:`!ASSERT2` expects 2 printf specifiers while :c:macro:`!ERROR` or :c:macro:`!ASSERT2` expects none).
+       [[noreturn]] void CELLO_ERROR(const char* format, ...);
+       void CELLO_REQUIRE(bool satisfies_requirement, const char* format, ...);
 
-    * The next arguments specifiy the variables used by the formatting specifier (if there are any).
+    The ``format`` argument is a :c:func:`!printf`-style formatting string and the trailing arguments specify that data to be formatted.
+    In the case of :c:macro:`!CELLO_REQUIRE`, the program will only abort if the ``satisfies_requirement`` argument is ``false``.
 
-    * The :c:macro:`!ASSERT` macro-family expects 1 last argument: the boolean condition dictating whether the program aborts.
+  .. note::
+
+     The :c:macro:`!CELLO_ERROR` macro is a replacement for the :c:macro:`!ERROR`, :c:macro:`!ERROR1`, :c:macro:`!ERROR2`, ..., :c:macro:`!ERROR8` family of macros.
+     Likewise  :c:macro:`!CELLO_REQUIRE`, is a replacement for the :c:macro:`!ASSERT`, :c:macro:`!ASSERT1`, :c:macro:`!ASSERT2`, ..., :c:macro:`!ASSERT8` family of macros.
+
+     The arguments for the older macros are similar, but slightly different.
+     The first argument is the name of the function where the macro is invoked.
+     The second argument is the format-string.
+     The next ``N`` arguments specifiy the data to be formatted (if ``N > 0``, the number is included in the macro name).
+     Members of the :c:macro:`ASSERT` macro-family each take 1 final argument: ``satisfies_requirement``.
 
 ====================
 Accessing Field data

doc/source/devel/debugging.rst

@@ -10,7 +10,7 @@ General Advice
 In general, if you find simple calls to ``CkPrintf`` are inadequate, you should consider whether the GDB debugger can help.
 A common scenario where the GDB debugger is useful is when you encounter a segmentation fault (this can occur when you dereference a ``NULL`` pointer).
 Essentially, the debugger runs the program until it crashes and then it will let you inspect variables in the stack frames just before the segmentation fault occurred.
-It can also be very useful to inspect variables in stack frames when the program aborted early during a call to the ``ERROR`` or ``ASSERT`` macros.
+It can also be very useful to inspect variables in stack frames when the program aborted early during a call to the ``CELLO_ERROR`` or ``CELLO_REQUIRE`` macros (or the older ``ERROR`` or ``ASSERT`` macros).
 
 Debugging a program with GDB is easiest when debugging a problem: replicated in a version of Enzo-E/Cello that was built on top of a netlrts-based build of charm++ on a local machine with xterm windows.
 In this scenario, you simply need to append ``++debug-no-pause`` to the arguments of the ``charmrun`` launcher (e.g. near the arguments specifying the number of nodes).

doc/source/devel/eos-fluidprops.rst

@@ -272,8 +272,7 @@ In this situation let's imagine that we want to write a more detailed error mess
       = enzo::fluid_props()->eos_variant().get_if<EnzoEOSIdeal>();
 
     if (eos == nullptr) {
-      ERROR("my_func",
-            "my_func only works when Enzo-E is configured with an ideal EOS");
+      CELLO_ERROR("my_func only works when Enzo-E is configured with an ideal EOS");
     }
     enzo_float gamma = eos->get_gamma();
     // do work with gamma...
@@ -299,8 +298,7 @@ Alternatively we could also accomplish the above by writing:
 
     const EnzoEOSVariant& eos_variant = enzo::fluid_props()->eos_variant();
     if (!eos_variant.holds_alternative<EnzoEOSIdeal>()) {
-      ERROR("my_func",
-            "my_func only works when Enzo-E is configured with an ideal EOS");
+      CELLO_ERROR("my_func only works when Enzo-E is configured with an ideal EOS");
     }
     enzo_float gamma = eos_variant().get<EnzoEOSIdeal>().get_gamma();
     // do work with gamma...
@@ -447,7 +445,7 @@ Now the obvious way to write this is:
      } else if (eos_variant.holds_alternative<EnzoEOSIsothermal>()) {
        return eos_variant.get<EnzoEOSIsothermal>().is_barotropic();
      } else {
-       ERROR("is_barotropic_eos", "eos_variant holds an unknown eos");
+       CELLO_ERROR("eos_variant holds an unknown eos");
      }
    }
 

src/Cello/error_Error.cpp

@@ -12,7 +12,7 @@
 
 //----------------------------------------------------------------------
 
-extern void cello::message
+void cello::message
 (
  FILE *       fp,
  const char * type,

src/Cello/error_Error.hpp

@@ -15,6 +15,111 @@
 /// @brief    Maximum length of error and warning messages
 #define ERROR_LENGTH 255
 
+//----------------------------------------------------------------------
+/// @def      __CELLO_PRETTY_FUNC__
+/// @brief    a magic contant like __LINE__ or __FILE__ used to specify the name
+///           of the current function
+///
+/// In more detail:
+/// - The C++11 standard ensures __func__ is provided on all platforms, but it
+///   only provides limited information (just the name of the function).
+/// - note that __func__ is technically not a macro. It's a static constant
+///   string implicitly defined by the compiler within each function definition
+/// - Where available, we prefer to use compiler-specific features that provide
+///   more information about the function (like the scope of the function, the
+///   the function signature, any template specialization, etc.).
+#ifdef __GNUG__
+  #define __CELLO_PRETTY_FUNC__ __PRETTY_FUNCTION__
+#else
+  #define __CELLO_PRETTY_FUNC__ __func__
+#endif
+
+//----------------------------------------------------------------------
+/// @def CELLO_ERROR
+/// @brief function-like macro that handles a (lethal) error message
+///
+/// This macro should be treated as a function with the signature:
+///
+///   [[noreturn]] void CELLO_ERROR(const char* msg, ...);
+///
+/// The ``msg`` arg is printf-style format argument specifying the error
+/// message. The remaining args arguments are used to format error message
+///
+/// @note
+/// the ``msg`` string is part of the variadic args so that there is always
+/// at least 1 variadic argument (even in cases when ``msg`` doesn't format
+/// any arguments). There is no portable way around this until C++ 20.
+///
+/// @note
+/// This is intended to replace the ERROR macros. There are 2 improvements:
+/// 1. CELLO_ERROR automatically detects the function name
+/// 2. CELLO_ERROR automatically takes a variable number of message-formatting
+///    arguments (in contrast, different ERROR macros are defined to accept
+///    different numbers of arguments)
+#define CELLO_ERROR(...)                                                      \
+  { cello::message                                                            \
+        (stderr,"ERROR",__FILE__,__LINE__,__CELLO_PRETTY_FUNC__,__VA_ARGS__); \
+        cello::error(); }
+
+//----------------------------------------------------------------------
+/// @def CELLO_REQUIRE
+/// @brief implements functionality analogous to the assert() macro
+///
+/// if the condition is false, print an error-message (with printf
+/// formatting) & abort the program.
+///
+/// This macro should be treated as a function with the signature:
+///
+///   void CELLO_REQUIRE(bool cond, const char* msg, ...);
+///
+/// - The 1st arg is a boolean condition. When true, this does nothing
+/// - The 2nd arg is printf-style format argument specifying the error message
+/// - The remaining args arguments are used to format error message
+///
+/// @note
+/// The behavior is independent of the ``NDEBUG`` macro
+///
+/// @note
+/// This is intended to replace the ASSERT macros. There are 3 differences:
+/// 1. CELLO_REQUIRE expects the boolean condition to be the first arg (rather
+///    than the last).
+/// 2. CELLO_REQUIRE automatically detects the function name
+/// 2. CELLO_REQUIRE automatically takes a variable number of message-formatting
+///    arguments (in contrast, different ASSERT macros are defined to accept
+///    different numbers of arguments)
+#define CELLO_REQUIRE(cond, ...)                                              \
+  {  if (!(cond))                                                             \
+      { cello::message                                                        \
+        (stderr,"ERROR",__FILE__,__LINE__,__CELLO_PRETTY_FUNC__,__VA_ARGS__); \
+        cello::error(); } }
+
+//----------------------------------------------------------------------
+/// @def CELLO_WARNING
+/// @brief function-like macro that handles a (non-lethal) warning message
+///
+/// This macro should be treated as a function with the signature:
+///
+///   void CELLO_WARN(const char* msg, ...);
+///
+/// The ``msg`` arg is printf-style format argument specifying the error
+/// message. The remaining args arguments are used to format error message
+///
+/// @note
+/// the ``msg`` string is part of the variadic args so that there is always
+/// at least 1 variadic argument (even in cases when ``msg`` doesn't format
+/// any arguments). There is no portable way around this until C++ 20.
+///
+/// @note
+/// This is intended to replace the WARNING macros. There are 2 improvements:
+/// 1. CELLO_WARN automatically detects the function name
+/// 2. CELLO_WARN automatically takes a variable number of message-formatting
+///    arguments (in contrast, different ERROR macros are defined to accept
+///    different numbers of arguments)
+#define CELLO_WARN(...)                                                       \
+  { cello::message                                                            \
+      (stderr,"WARNING",__FILE__,__LINE__,__CELLO_PRETTY_FUNC__,              \
+       __VA_ARGS__); }
+
 //----------------------------------------------------------------------
 /// @def      WARNING
 /// @brief    Handle a (non-lethal) warning message
@@ -47,6 +152,9 @@
       (stdout,"WARNING",__FILE__,__LINE__,F,M,A1,A2,A3,A4,A5,A6,A7,A8); }
 
 //----------------------------------------------------------------------
+
+// these macros are deprecated. Prefer to use the CELLO_ERROR macro
+
 /// @def      ERROR
 /// @brief    Handle a (lethal) error message
 #define ERROR(F,M)                              \
@@ -256,6 +364,10 @@
 #endif /* CELLO_DEBUG */
 
 //----------------------------------------------------------------------
+
+// these macros are deprecated. New code should prefer the CELLO_REQUIRE macro.
+// Note that the CELLO_REQUIRE macro expects the condition as the first arguement
+
 /// @def      ASSERT
 /// @brief    Equivalent to assert()
 
@@ -314,7 +426,7 @@
 
 
 namespace cello {
-  extern void message
+  void message
   (FILE * fp,
    const char * type, 
    const char * file, 

src/Cello/test_Error.cpp

@@ -20,7 +20,7 @@ PARALLEL_MAIN_BEGIN
 
   unit_class("Error");
   //----------------------------------------------------------------------
-  PARALLEL_PRINTF ("Warning message:\n");
+  PARALLEL_PRINTF ("old-style Warning message:\n");
 
   char warning_message[ERROR_LENGTH];
   snprintf (warning_message,sizeof(warning_message),"Warning message test");
@@ -29,6 +29,16 @@ PARALLEL_MAIN_BEGIN
   unit_func("WARNING");
   unit_assert (true);
 
+  //----------------------------------------------------------------------
+  PARALLEL_PRINTF ("new-style Warning message:\n");
+
+  char warning_message2[ERROR_LENGTH];
+  snprintf (warning_message2,sizeof(warning_message2),"Warning message test");
+  CELLO_WARN("%s", warning_message2);
+
+  unit_func("WARNING");
+  unit_assert (true);
+
   //----------------------------------------------------------------------
   PARALLEL_PRINTF ("Incomplete message:\n");