Problematic implementation of LOG\LOGF macros

[omerah]

Hi,
There's a basic pitfall in the way LOG\LOGF macros were implemented:
#define LOG(level) if(g3::logLevel(level)) INTERNAL_LOG_MESSAGE(level).stream()
If we take the following code:
if (x) LOG(INFO) << "This message"; else LOG(INFO) << "That message";

After the preprocessing stage we get:
if (x) if(g3::logLevel(level)) INTERNAL_LOG_MESSAGE(level).stream() << "This message"; else if(g3::logLevel(level)) INTERNAL_LOG_MESSAGE(level).stream() << "That message";

As you can see, the compiler has no way to know that the 'else' statement belongs to the first if statement and not the the second one (that comes from the definition of the LOG macro).

The correct way to write this macro would be:
#define LOG(level) if(!g3::logLevel(level)){\ } else INTERNAL_LOG_MESSAGE(level).stream()
Which will, after preprocessing, turn into:
if (x) if(!g3::logLevel(level)){ } else INTERNAL_LOG_MESSAGE(level).stream() << "This message"; else if(!g3::logLevel(level)){ } else INTERNAL_LOG_MESSAGE(level).stream() << "That message";

Which keeps the original logic we're interested in.

[KjellKod]

Looks good. Please put up a pull request

[KjellKod]

@omerah : have you considered opening up a pull request for this change?

[maj-tom]

The dangling else strikes again :)
I'll try to find time this weekend to do this as well as fix the test fails in the PR I sent before (started a new job so I've been overwhelmed).

[KjellKod]

Candidate for closing unless a PR shows up. @omerah?

[omerah]

Hi, Sorry, I am pretty busy, don’t have time to fix it at the moment :( Thanks, Omer. From: Kjell Hedström [mailto:notifications@github.com] Sent: Wednesday, October 25, 2017 11:20 PM To: KjellKod/g3log Cc: Omer Aharonian; Mention Subject: Re: [KjellKod/g3log] Problematic implementation of LOG\LOGF macros (#224) Candidate for closing unless a PR shows up. @omerah<https://github.com/omerah>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#224 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/Ad_RH-1pMXED3IQb90NJfcIwT4Qqbfd8ks5sv5f6gaJpZM4PGEnu>.

[KjellKod]

@maj-tom

Many thanks!