Negating expression in if statement inside macro gives odd results

Question

I've run into a somewhat strange issue. It makes me feel like the answer is blaringly obvious and I'm just not seeing something because the code is so simple.

I basically have a macro called "ASSERT" that makes sure a value isn't false. If it is, it writes a message to the console and debug breaks. My issue is that when asserting that the index returned from std::string::find_first_of(...) is not equal to std::string::npos, the assert doesn't seem to work at all. Every time, the assertion fails.

I have verified that the values are not equal when the debug break occurs, so I don't see how the assertion is failing.

In my original code, it reads data from a file to a string, but the issue still seems to be present in the example below with nothing but a const std::string.

I'm working on a much bigger project, but here's a minimal example that reproduces the bug (I'm using Visual Studio 2022 and C++17 by the way):

#include <iostream>
#include <string>

#define ASSERT(x, msg) {if(!x) { std::cout << "Assertion Failed: " << msg << "\n"; __debugbreak(); } }

int main() {

    const std::string source = "Some string that\r\n contains the newline character: \r\n...";

    size_t eol = source.find_first_of("\r\n", 0);
    ASSERT(eol != std::string::npos, "Newline not present!");
    
    // Other code...
    
    return 0;
}

Please note that the exact same thing happens even when only one newline character ("\r\n") is present in the string.

What's interesting is that "eol" seems to have the correct value in every test case that I've run. The only thing that's wrong is the assertion, so if I ignore it and continue, everything runs exactly how I expect it to.

I also found this issue that seems related, but no answer or conclusion was reached: std::string::find_first_of does not return the expected value

Answer 1

An answer has been provided in the comments. The solution is to simply add parenthesis around the x in the "!x" portion of the macro. This translates to the following (credit to 0x5453):

#define ASSERT(x, msg) {if(!(x)) { ... } }

The reason that this fixes the problem is that if you expand the original macro, it'll look something like this (this is how it is compiled):

if(!eol != std::string::npos) {...}

This is pretty obviously incorrect because you want to check that the entire condition is false. Like this:

if(!(eol != std::string::npos)) {...}

Adding the parentheses fixes this.

Simple, but still a good reminder to check the little things like this.

Answer 2

This is the unexpected consequence of the by-design simple stupidity of the preprocessor's macro substitution engine. An expression supplied to the macro is not evaluated as it would be with a function, the text is inserted directly during substitution.

Given

#define ASSERT(x, msg) {if(!x) { std::cout << "Assertion Failed: " << msg << "\n"; __debugbreak(); } }

the line

ASSERT(eol != std::string::npos, "Newline not present!");

will be transformed into

{if(!eol != std::string::npos) { std::cout << "Assertion Failed: " << "Newline not present!" << "\n"; __debugbreak(); } }

and the ! is only applied to the eol, changing the expected behaviour of the macro to something nonsensical.

Adding the extra brackets recommended in the comments

#define ASSERT(x, msg) {if(!(x)) { std::cout << "Assertion Failed: " << msg << "\n"; __debugbreak(); } }

results in

{if(!(eol != std::string::npos)) { std::cout << "Assertion Failed: " << "Newline not present!" << "\n"; __debugbreak(); } }

and now the expression is being evaluated before applying the ! and tested.

Because macros are "evil", and since the macro makes no use of any special, position dependent debugging macros like __FILE__ or __LINE__, this is a case where I would replace the macro with a function and count on the compiler's optimizations to inline it.