Introduce CELLO_ERROR, CELLO_REQUIRE, CELLO_WARN

[mabruzzo]

Pull request summary

This introduces 3 function-like macros for error-reporting and logging purposes. The intention is to replace the existing ERROR, ASSERT and WARNING families of macros.

Detailed Description

About CELLO_ERROR

For the sake of motivating this change, let's focus on the CELLO_ERROR macro. This macro should be treated as though it is a function with the declaration

[[noreturn]] void CELLO_ERROR(const char* format, ...);

The idea is that this single macro replaces the entire exiting ERROR macro family. For context, those macros are used as though they have the following function signatures:

// in this snippet,  T1, T2, ... are just used as undeclared place-holder types.
[[noreturn]] void ERROR(const char* func_name, const char* format);
[[noreturn]] void ERROR1(const char* func_name, const char* format, T1 arg1);
[[noreturn]] void ERROR2(const char* func_name, const char* format, T1 arg1, T2 arg2);
// skipping ERROR3, ERROR4, ERROR5, ERROR6, and ERROR7
[[noreturn]] void ERROR8(const char* func_name, const char* format, T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5, T6 arg6, T7 arg7, T8 arg8);

Essentially, CELLO_ERROR makes 2 key innovations:

1. we no longer need to specify func_name, the name of the function that we are invoking the macro from. This is detected automatically by CELLO_ERROR
   • we can detect the function name by using the __func__ variable that is implicitly define within every function/method starting with c++ 11¹
   • this is important. People sometimes currently forget to manually change func_name when they copy an error between functions or they rename a function. This change will make things completely unnecessary.
2. We no longer have separate macros for different numbers of formatting arguments.

Other macros

Just as CELLO_ERROR replaces the ERROR macro family,

• CELLO_REQUIRE replaces the ASSERT macro family
• CELLO_WARN replaces the WARNING macro family

These other 2 macros should be treated as though they are functions with the following signatures

void CELLO_REQUIRE(bool satisfies_requirement, const char* format, ...);
void CELLO_WARN(const char* format, ...);

One important note: in the ASSERT macro-family, the assertion condition is always the last argument. In contrast, the assertion-condition is always the first argument of CELLO_REQUIRE.

• This difference is just a consequence of how variadic macros work.
• It's also part of the reason I named the replacement CELLO_REQUIRE rather than CELLO_ASSERT.²

Caveats

• The main caveat is that these new macros will coexist alongside the old macros.

  • From some quick shell commands, it appears that there are ~1000 occurrences of the old macros. I don't really have any interest in systematically going through and replacing the old macros (I could probably be convinced to remove the versions that took more than say 5 arguments, I think that would be tractable)
  • I personally think that the utility of these new commands are significant enough to warrant them existing side-by-side.
  • But if any reviewer (or anybody in general) disagrees, I'm totally fine with that. We can just close the PR.

• In the event that this is accepted, I think we should hold off on merging this until after we make a decision on Issue Use fmt C++ library? OPINIONS WANTED! #400 (to be clear, I think the review of this PR should proceed without worrying about that other issue - we may just want to coordinate things if we decide to proceed with both proposals).

This is a major change or addition (that needs 2 reviewers): unknown

PR Checklist

• New features are documented with narrative docs.

Footnotes

1. where available, we prefer compiler-specific alternatives like __PRETTY_FUNCTION__ on gcc and clang. This gives a more descriptive name (including the scope, any template specializations, and the precise function signature). ↩

2. the other reason is to make CELLO_REQUIRE a little more distinct from the assert macro introduced by <assert.h>. The key point is that the behavior of CELLO_REQUIRE is completely independent of other macros. In contrast, assert becomes a no-op when the NDEBUG macro is defined. ↩

[jobordner]

Overall very nice!
Would it be difficult to keep CELLO_ASSERT in addition to CELLO_REQUIRE? My original intention was to use CELLO_ASSERT for checking code during development, and allow it to be disabled in production code. CELLO_REQUIRE would be something you'd want enabled all the time, hence having both. I think the main difficulty would be going through the existing ones and deciding which one each should be, though that can be deferred.