Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings about REQUIRE when used in templates #565

Closed
rr- opened this issue Dec 28, 2015 · 5 comments
Closed

Warnings about REQUIRE when used in templates #565

rr- opened this issue Dec 28, 2015 · 5 comments
Labels

Comments

@rr-
Copy link

rr- commented Dec 28, 2015

Consider this:

template<class T> inline void test_reading_multiple_bytes(const TestType type)
{
    SECTION("...")
    {
        T reader("\x8F\x8F"_b); // 10001111
        if (type == TestType::Msb)
        {
            REQUIRE((reader.get(7) == (0x8F >> 1)));
            REQUIRE(reader.get(1));
            REQUIRE(reader.get(1));
            REQUIRE(!reader.get(1));
            REQUIRE((reader.get(4) == 3));
            REQUIRE((reader.get(2) == 3));
        }
        else
        {
            REQUIRE(reader.get(1));
            REQUIRE((reader.get(7) == (0x8F >> 1)));
            REQUIRE(reader.get(1));
            REQUIRE(reader.get(1));
            REQUIRE(reader.get(2) == 3);
            REQUIRE(reader.get(4) == 8);
        }
    }
}

TEST_CASE("...")
{
    test_reading_multiple_bytes<MsbBitReader>(TestType::Msb);
    test_reading_multiple_bytes<LsbBitReader>(TestType::Lsb);
}

This produces warnings with gcc 5.3 on GNU/Linux:

/home/rr-/src/arc_unpacker/tests/io/bit_reader_test.cc: In instantiation of ‘void test_reading_multiple_bytes({anonymous}::TestType) [with T = au::io::OldBitReader]’:
/home/rr-/src/arc_unpacker/tests/io/bit_reader_test.cc:277:64:   required from here
/home/rr-/src/arc_unpacker/tests/io/bit_reader_test.cc:89:39: warning: suggest parentheses around comparison in operand of ‘==’ [-Wparentheses]
             REQUIRE(reader.get(2) == 3);
                                   ^
/home/rr-/src/arc_unpacker/tests/io/bit_reader_test.cc:90:39: warning: suggest parentheses around comparison in operand of ‘==’ [-Wparentheses]
             REQUIRE(reader.get(4) == 8);

When I wrap with ( ) like gcc asks me to, it stops complaining, although on top of code becoming ugly, I lose information on the reason behind failed tests (i.e. rather than being shown useful 3 == 8, I am shown false only - not useful much).

@rr-
Copy link
Author

rr- commented Dec 28, 2015

...and in case you wonder - the reason behind templated tests is basically rolling my own solution to #357.

@horenmar
Copy link
Member

Since we recently improved supression of -Wparentheses I though I could just close this, but as it turns out, g++ will not warn on the same code outside of templates, and will not accept _Pragma based supression inside of templates.

@philsquared
Copy link
Collaborator

The current workaround for this issue is to suppress the warning for your whole test file (using the appropriate pragma for your compiler)

@bdb
Copy link

bdb commented Mar 16, 2017

Just another vote for fixing this. I have a few templated tests that worked fine in 1.7 and 1.6 but are now broken in 1.8.2 (with GCC 5.4).

As a workaround I just wrapped the template definition with the CATCH_INTERNAL... macros instead of setting the pragma for the whole file.

@horenmar
Copy link
Member

horenmar commented Apr 5, 2017

I am going to close this, as I moved Catch back to force disabling -Wparentheses everywhere using #pragma in 8f85d08 and this sample code compiles cleanly.

#define CATCH_CONFIG_MAIN
#include "catch.hpp"

template <typename T>
void foo(T) {
    int a, b;
    a = 1; b = 2;
    REQUIRE(a == b);
}

TEST_CASE("Wparentheses") {
    foo(true);
}

This means that we disable the warning for GCC users even outside our assertions, but the spotty _Pragma support meant that the status quo was untenable.

@horenmar horenmar closed this as completed Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants