Add slic macros that log once per call site#1831
Conversation
…test in slic_macros serial
| #define SLIC_DETAIL_LOG_IF_ONCE(macro, EXP, msg) \ | ||
| do \ | ||
| { \ | ||
| static bool once = true; \ | ||
| if(EXP) \ | ||
| { \ | ||
| if(once) \ | ||
| { \ | ||
| macro(true, msg); \ | ||
| once = false; \ | ||
| } \ | ||
| } \ | ||
| } while(axom::slic::detail::false_value) |
There was a problem hiding this comment.
Helper macro that the SLIC_*_ONCE macros use.
| // Convenience test macro that checks slic macro has logged a message | ||
| #define EXPECT_SLIC_LOG(macro_call, level, message) \ |
There was a problem hiding this comment.
Convenience macro for testing, mainly to alleviate the tracking of __LINE__ in the original tests. It works pretty well for gcc, except...
There was a problem hiding this comment.
We can use std::source_location when we move to c++20 --- https://en.cppreference.com/w/cpp/utility/source_location.html
| // Single line - Placement of ")" matters for __LINE__ for slic call and checking | ||
| // clang-format off | ||
| EXPECT_SLIC_LOG(SLIC_ERROR_IF(true, "this message is logged!"), "ERROR", "this message is logged!"); | ||
| // clang-format on |
There was a problem hiding this comment.
... clang and intel compilers have different __LINE__ behavior, I need to disable formatting here to keep the entire call to one line.
Otherwise, the observed behavior for example is something like:
EXPECT_SLIC_LOG(
SLIC_ERROR_IF(true, "this message is logged!"), // <-- This is the line number that is logged
"ERROR",
"this message is logged!"); // <-- This is the line number the macro is checking for
The line number as far as I can tell in both instances is actually determined by the line placement of the closing parantheses ")".
Another example for future reference:
EXPECT_SLIC_LOG(SLIC_ERROR_ROOT_IF(true,
"this message is logged!"
), // <-- This is the line number that is logged
"ERROR",
"this message is logged!"
); // <-- This is the line number the macro is checking for
kennyweiss
left a comment
There was a problem hiding this comment.
Thanks for adding this @bmhan12
- Please reintroduce the
do { } while()to the macros - Let's ensure that we have the right semantics for
SLIC_*_IF_ONCE( false )
| if(EXP) \ | ||
| { \ | ||
| if(once) \ | ||
| { \ | ||
| macro(true, msg); \ | ||
| once = false; \ | ||
| } \ | ||
| } \ |
There was a problem hiding this comment.
Gut check -- this can call the macro multiple times, until EXP evaluates as true.
Is this the semantics we're looking for? Or should it only call the the macro once, independent of whether EXP is true/false?
There was a problem hiding this comment.
this can call the macro multiple times, until EXP evaluates as true.
I think this is the behavior we want, and is what was in the original #1683 snippet.
A once-only if-check I think would be false and print nothing most of the time, unless you knew the true condition ahead of time to guarantee the first invocation prints, in which case, your SLIC_*_IF_ONCE usage is more like SLIC_*_ONCE .
| // Convenience test macro that checks slic macro has logged a message | ||
| #define EXPECT_SLIC_LOG(macro_call, level, message) \ |
There was a problem hiding this comment.
We can use std::source_location when we move to c++20 --- https://en.cppreference.com/w/cpp/utility/source_location.html
Arlie-Capps
left a comment
There was a problem hiding this comment.
Thanks for your careful work, @bmhan12 .
| | | ``SLIC_WARNING_ROOT`` | | | | No longer collective after calling ``slic::disableAbortOnWarning()`` | | ||
| | | ``SLIC_WARNING_ROOT_IF`` | | | | | | ||
| +----------------------------+------------------------------------------------+----------------------------------------------------------------------------+ | ||
| .. list-table:: SLIC macro availability and collective behavior |
There was a problem hiding this comment.
@bmhan12 thank you for this. Much better way to make a table in sphinx -- avoid lining up all the characters across rows and columns 🥇
This PR:
SLIC_*_ONCEmacros that are invoked once per call site.Addresses #1685