0

Well, I've come across a strange problem.

Since adding the spdlog library to my project and adjusting all log output accordingly, all else blocks of my if-else statements are ignored. The log output has been replaced by a search-and-replace procedure with a simple regex pattern (printf-style to fmtlib-style).

spdlog: https://github.com/gabime/spdlog

After some debugging, I have now written the following very simple test function:

void dbg_test() {
    bool success = false;

    BOOST_ASSERT_MSG(!success, "Sanity check");

    // This works
    if (success) {
        LOG_ERROR("Successful");
    } else {
        LOG_ERROR("Unsuccessful"); // Log's written
    }

    // This doesn't work
    if (success)
        LOG_ERROR("Successful (w/o)");
    else
        LOG_ERROR("Unsuccessful (w/o)"); // Log's missing
}

Depending on the value of "success" the following now happens (BOOST_ASSERT_MSG is adapted accordingly): If success == true, "Successful" and "Successful (w/o)" is written to the log. If success == false, "Unsuccessful" is written, but "Unsuccessful (w/o)" is not.

Of course I also checked the logging and wanted to end the program with exit(0) instead of writing logs, but this code didn't work either:

void dbg_test() {
    bool success = false;

    BOOST_ASSERT_MSG(!success, "Sanity check");

    // This works
    if (success) {
        LOG_ERROR("Successful");
    } else {
        exit(0);
    }

    // This doesn't work
    if (success)
        LOG_ERROR("Successful (w/o)");
    else
        exit(0);
}

For compilation and debugging I use Microsoft Visual Studio Community 2017 (Version 15.5.0). The corresponding project folder was created with CMake 3.13.4.

What could be the reason for this?


Edit

Macro definition for LOG_ERROR:

#define LOG_ERROR(...) SPDLOG_ERROR(__VA_ARGS__)
Daniel B
  • 13
  • 2
  • 5
    What is `LOG_ERROR`? How is the macro (I assume) defined? – Some programmer dude Apr 04 '19 at 06:36
  • 2
    Almost certainly a poorly defined `LOG_ERROR` macro, but without seeing the macro definition who can be sure? – john Apr 04 '19 at 06:37
  • 5
    My crystal ball says that [this thread](https://stackoverflow.com/questions/154136/why-use-apparently-meaningless-do-while-and-if-else-statements-in-macros) will be useful – M.M Apr 04 '19 at 06:38
  • As I wrote below, I also checked this and replaced LOG_ERROR in the appropriate lines that should be handled with exit(0) (first for the code that doesn't work, then for the working one, otherwise I would never reach the non-functioning code). However, this is the definition of the macro: `#define LOG_ERROR(...) SPDLOG_ERROR(__VA_ARGS__)` – Daniel B Apr 04 '19 at 06:43
  • And what's `SPDLOG_ERROR`? You don't need to expand the whole thing yourself, you know. VC support a `/E` flag to preprocess only. – StoryTeller - Unslander Monica Apr 04 '19 at 06:48
  • @M.M I've read the thread you linked and am taking a look at it right now. Obviously that could be the reason why even my second code failed (as I still use macros in the if-block). – Daniel B Apr 04 '19 at 06:48
  • @StoryTeller Yep, it seems like this is the problem. I didn't know about this case of error source. SPDLOG_ERROR expands to `if (spdlog::default_logger_raw()->should_log(spdlog::level::err)) \ spdlog::default_logger_raw()->log(spdlog::source_loc{SPDLOG_FILE_BASENAME(__FILE__), __LINE__, SPDLOG_FUNCTION}, spdlog::level::err, __VA_ARGS__)` which contains an if-statement already. That seems to be the problem. – Daniel B Apr 04 '19 at 06:52
  • No, just accept an answer – M.M Apr 04 '19 at 06:54
  • Pretty bad mistake from what purports to be modern style library. – M.M Apr 04 '19 at 06:55
  • Yes, I'm currently creating an issue there to tell them about that error. I will try to fix it myself later but I'm not that convenient with GitHub fork, pull-requests, etc. – Daniel B Apr 04 '19 at 06:59
  • Downvote reversed. This question is very clear - the problem can only be in the macro. – Bathsheba Apr 04 '19 at 07:26

1 Answers1

5

Lot's of fancy code but they can't get a simple macro right. This isn't the LOG_ERROR macro (I couldn't find that) but it illustrates the problem

#define SPDLOG_LOGGER_CALL(logger, level, ...) \
    if (logger->should_log(level)) \
        logger->log(spdlog::source_loc{SPDLOG_FILE_BASENAME(__FILE__), __LINE__, SPDLOG_FUNCTION}, level, __VA_ARGS__)

should be

#define SPDLOG_LOGGER_CALL(logger, level, ...)  \
    do \
        if (logger->should_log(level)) \
            logger->log(spdlog::source_loc{SPDLOG_FILE_BASENAME(__FILE__), __LINE__, SPDLOG_FUNCTION}, level, __VA_ARGS__) \
    while (false)

The do ... while trick ensures that the output of the macro can be used as a simple statement. That's not true of the if statement that the original macro output which, as you found, is going to mess up any if ... else statement it finds itself in.

Maybe you could join the project and push this fix.

john
  • 85,011
  • 4
  • 57
  • 81
  • Well based on the comments above this seems to be the explanation. LOG_ERROR expands to SPDLOG_ERROR which expands to SPDLOG_LOGGER_CALL – john Apr 04 '19 at 06:50