Fix ATF_REQUIRE_EQ() for gcc 4.9

[rodrigc]

The fix is to cast the NULL value to the specific pointer type ATF is testing against.

In C++ mode, ATF_REQUIRE_EQ will attempt to output the value of NULL onto a std::ostringstream, but this has become ambiguous in C++11.

Before C++11, NULL is defined as 0, e.g. plain zero, and it will therefore be printed as an integer "0". In C++11 and later, NULL is defined as the keyword nullptr, e.g. the null pointer literal. Since the null pointer can be converted to any other pointer type, and there are many different operators defined in C++ to output those operators, the call is ambiguous, and must be resolved by the developer.

Submitted by: Dimitry Andric dim@FreeBSD.org

[rodrigc]

@DimitryAndric: This is the patch which you submitted here: https://lists.freebsd.org/pipermail/freebsd-current/2015-March/054935.html Unfortunately, with this patch, the Travis build of ATF fails.

Do you have an idea for another patch which fixes the problem?

[DimitryAndric]

@rodrigc, the patch that Travis has tested seems to be the one I posted in a follow up message, and which casts the 'actual' parameter to the type of 'expected', e.g.:

        atfu_ss << "Line " << __LINE__ << ": " \
                << #expected << " != " << #actual \
                << " (" << (expected) << " != " << \
                static_cast<__typeof(expected)>(actual) << ")"; \

However, having thought a little about this, it is not the correct approach for the general case, as the types of 'expected' and 'actual' are not necessarily equal.

The original problem posted on freebsd-current was about the tests in lib/libnv causing errors with -std=c++11, and I think this should be solved in libnv instead. E.g. whenever ATF_REQUIRE_EQ is used to compare against NULL, the NULL should be static_cast'd to the correct type.

This is already done in atf itself; for example in atf-c++/detail/process_test.cpp:

    ATF_REQUIRE(std::strcmp(eargv2[0], carray1[0]) == 0);
    ATF_REQUIRE_EQ(eargv2[1], static_cast< const char* >(NULL));

and similar in other places.

[jmmv]

I kinda agree with Dimitry here.

I'm wary of the original proposed patch as I fear the static casting has chances of unpredictably breaking stuff. One reason is because even though the macro recommends the (expected, actual) order, there is nothing to enforce that and thus, while casting "actual" to "expected" may be OKish in the general case, there is code out there that does not respect the ordering and thus you'd be casting "expected" to "actual". (Mind you, the legacy code in ATF and Kyua use the "old" (actual, expected) ordering.)

The reason the Travis CI build is broken is precisely because of the above. There are many calls to ATF_REQUIRE comparing character arrays to std::string objects. While automated type promotion does the right thing here, static_cast does not if provided in the "wrong order".

I suspect that if you use nullptr instead of NULL in the tests, the problems will go away. Or we should fix the tests instead to static_cast NULL to the right type.

[DimitryAndric]

I suspect that if you use nullptr instead of NULL in the tests, the problems will go away.

Actually, this is precisely what is causing the problem: before C++11, the macro NULL is defined as 0, but in C++11 and later, it is defined as nullptr. It is the latter that is causing the trouble, since previously 0 could easily be output to std::ostream (as it is just an integer), whereas nullptr cannot.

Or we should fix the tests instead to static_cast NULL to the right type.

As far as I could see, in ATF itself, this is already done correctly everywhere. At least, I tried cloning master off GitHub, and building it with clang and -std=c++11, and there were no errors.

For FreeBSD's libnv, I have submitted https://reviews.freebsd.org/D2027.

[jmmv]

Oh OK. I somehow missed the fact that nullptr cannot be output to std::ostream. That's ugly though! Wondering why this is forbidden.

And yep, I meant fixing the FreeBSD tests (as you suggested).

Based on the above, I think we can close this issue. Doing so now. Please reopen if you think something else needs to be done.

[DimitryAndric]

I somehow missed the fact that nullptr cannot be output to std::ostream. That's ugly though! Wondering why this is forbidden.

It is not forbidden, just ambiguous, since there exist implicit conversions from nullptr to null pointer values of any pointer type and any pointer to member type. Therefore, you must always indicate the exact pointer type you are intending.

[rodrigc]

OK, thanks for the analysis.