0

I have a logging macro that is made of multiple functions and i looks like this:

#define MY_LOG(s) { myMutex.lock(); std::cout << __PRETTY_FUNCTION__ << " (" << __LINE << ")" << s << std::endl; myMutex.unlock(); }

I want to be able to call the macro so it looks similar to calling a normal function. I want to have it beeing terminated by a semicolon.

It does work in most cases. Here is an example:

if (1 == 1)
    MY_LOG("ok " << 1);

It works fine. No problem. But it does not work in this case:

if (1 == 1)
    MY_LOG("1 == 1");
else
    c++;

I get an error that there is an else without previous if. Is it possible to fix this?

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
zomega
  • 1,538
  • 8
  • 26
  • 4
    [Surround the macro body by a `do { ... } while (0)` statement](https://stackoverflow.com/questions/154136/why-use-apparently-meaningless-do-while-and-if-else-statements-in-macros). – Some programmer dude May 19 '22 at 13:55
  • 1
    With that said, I recommend a somewhat different design to your logging system: Define a macro that creates an object of a class whose constructor does all the initialization needed (like locking etc.) and whose destructor cleans up (unlocks etc.). It could have overloads for the `<<` operator for the logging (similar to how `std::cout` works). Then you could do something like `LOG() << "foo" << bar;`. The destructor will of course flush the log output to make sure output is actually written. – Some programmer dude May 19 '22 at 13:58

2 Answers2

2

The reason that it works at all is that an extra ; will just be ignored by the compiler in most cases. What you are writing is { ... };.

Not so for if/else. Consider this code:

if (cond1)
    if (cond2) something();
else {
    ...
}

If the extra ; would not close the inner if then you would have to add else { }. The designer of C/C++ went with using ; as a shorter version of that.


Now to fix your macro you have to end in something that requires a ; but had otherwise no effect. The solution to this is Why use apparently meaningless do-while and if-else statements in macros?

#define mymacro(parameter) do{/**/} while(0)

But the whole macro is obsolete in modern (since c++20) C++ with the introduction of std::source_location. Just make it an actual function:

void log(const std::string_view message,
         const std::source_location location = 
               std::source_location::current())
{
    std::lock_guard<std::mutex> lock(myMutex);
    std::cout << "file: "
              << location.file_name() << "("
              << location.line() << ":"
              << location.column() << ") `"
              << location.function_name() << "`: "
              << message << '\n';
}
Goswin von Brederlow
  • 11,875
  • 2
  • 24
  • 42
  • I don't think I follow your first example. If you wrote it as `if(cond1) { if (cond2) { ... } } else { ... }` with no semicolons, wouldn't the fact that the entire `cond2` conditional is inside the compound statement still mean that the `else` attaches to `cond1` and not `cond2`? – Nathan Pierson May 19 '22 at 14:19
  • 1
    @NathanPierson Sorry, too many {}, corrected the code. It's just so ingrained to always use `{}` for any if with a linebreak for me. – Goswin von Brederlow May 19 '22 at 14:23
1

The usual solution is to use do { ... } while(0) like this:

#define MY_LOG(s) do {                                                            \
        std::lock_guard<std::mutex> lock(myMutex);                                \
        std::clog << __PRETTY_FUNCTION__ << " (" << __LINE__ << ")" << s << '\n'; \
    } while(false)

Note the std::lock_guard instead of manual locking.

But with a variadic macro, a variadic function template and C++20 std::source_location you can make something far more versatile:

#include <iostream>
#include <mutex>
#include <sstream>
#include <source_location>

template<class... Args>
void my_log(const std::source_location loc, Args&&... args)
{
    static std::mutex myMutex;
    std::ostringstream os;      // used to build the logging message

    os << loc.function_name() << " (" << loc.line() << "):" << std::boolalpha;

    // fold expression:
    ((os << ' ' << args), ...); // add all arguments to os
    
    std::lock_guard<std::mutex> lock(myMutex); // use a lock_guard
    std::clog << os.str();                     // and stream
}

// this just forwards the source location and arguments to my_log:
#define MY_LOG(...) my_log(std::source_location::current(), __VA_ARGS__)

int main() {
    MY_LOG("A", 1, 2, 'B');
}

Demo

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108