warning generated using Catch in g++-4.7: suggest parentheses around comparison in operand of '=='

[jcrada]

Hi,

The following warning is generated in my project using Catch in GNU g++-4.7:

/build/fuzzylite/test/BenchmarkTest.cpp: In function 'void fl::____C_A_T_C_H____T_E_S_T____32()':
/build/fuzzylite/test/BenchmarkTest.cpp:85:13: error: suggest parentheses around comparison in operand of '==' [-Werror=parentheses]

It would be great to be able to compile the tests without generating such a warning.

Project: https://github.com/fuzzylite/fuzzylite/tree/master
Commit: https://github.com/fuzzylite/fuzzylite/tree/d095efce076549b435ea5f9e21addf8a7346965c
Travis failing job: https://travis-ci.org/fuzzylite/fuzzylite/jobs/137058960

Thanks for such a great testing framework!

[czipperz]

Link to the actual code as well please: https://github.com/fuzzylite/fuzzylite/blob/d095efce076549b435ea5f9e21addf8a7346965c/fuzzylite/test/BenchmarkTest.cpp#L85

That == true should be deleted, it is invalid if I give you any truthy value instead of true.

[jcrada]

Thank you very much for your feedback. Unfortunately, there are other cases where I am being required to enclose in parentheses different kinds of statements. For example,

https://github.com/fuzzylite/fuzzylite/blob/d095efce076549b435ea5f9e21addf8a7346965c/fuzzylite/test/activation/ThresholdTest.cpp#L31

https://github.com/fuzzylite/fuzzylite/blob/d095efce076549b435ea5f9e21addf8a7346965c/fuzzylite/test/activation/ThresholdTest.cpp#L36

and so on, thereby requiring checks in the form CHECK(( a == b )) to use double parentheses in order to avoid triggering a warning. It seems a bit silly to have to use double parenthesis in such cases to avoid a warning.

Would it not be possible to do such an enclosing in Catch instead? Hence, addressing this issue in g++-4.7 and possibly earlier versions of the compiler?

Thanks again!

[czipperz]

I'll try rolling back g++ on my arch box and looking at that. I'm by no means a Catch developer, I'm just trying to help if I can.

[jcrada]

Thanks for your help!

You could use my .travis.yml and Dockerfile file scripts to configure Docker to run with g++-4.7, without needing to roll back in your box.

Just clone my repository in master branch (https://github.com/fuzzylite/fuzzylite/tree/master), and issue the following commands:

export CXX_COMPILER=g++-4.7
docker build -t fuzzylite -f Dockerfile --build-arg CXX_COMPILER=${CXX_COMPILER} .
docker run -e CXX=${CXX_COMPILER} -e FL_CPP11=${FL_CPP11} -e FL_USE_FLOAT=${FL_USE_FLOAT} -e FL_BUILD_TESTS=ON --entrypoint /bin/bash -it fuzzylite 

The docker run will open bash for you on my projects root, where you can issue build.sh release to build the release of the software.

Thanks!

Let me know if I can be of any help.

[philsquared]

We can't put it in parentheses within the macro as this defeats the expression decomposition (so you won't get LHS and RHS values separately).

I use this to suppress the same warning with clang:

#define CATCH_INTERNAL_SUPPRESS_PARENTHESES_WARNINGS _Pragma( "clang diagnostic ignored \"-Wparentheses\"" )

Is there a "gcc diagnostic ignored" version? And if so from what version is it available? I'm happy to put that in but I don't have easy access to gcc at the moment to test it.

[czipperz]

https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

Same thing but replace clang with GCC

[czipperz]

Ha -> Arch Linux Archives don't go back to g++ 4.7!

[onqtam]

_Pragma() will not work for g++ (only gcc) in complex macros - see this - in my library doctest I'm able to suppress only clang warnings

[czipperz]

Can we just wrap the entire header in a #pragma push pop for this?

[onqtam]

@czipperz if you pop the silenced warning at the bottom of the header - it will not affect code generated from a macro in a source file - so it either has to use _Pragma() or it has to be left enabled even after the header

[jcrada]

Many thanks for your help!
Since the warning is raised only in g++-4.7, I decided to ignore the warning from the CMakeLists.txt by adding the following:

 if ("${CMAKE_CXX_COMPILER_ID}"  STREQUAL "GNU") 
    if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.8)
        set_target_properties(fl-test PROPERTIES COMPILE_FLAGS "-Wno-parentheses")
    endif()
endif()

I am happy to have this issue closed.

Thanks.

[czipperz]

-Wno-error=parenthesis is another way

[vadz]

FWIW this is still a problem with v1.7.1. To be precise, it's a problem with the g++ versions from 4.4 to 4.7, inclusive. There are no warnings with 4.2 (I couldn't find 4.3 to test) nor with 4.8, 4.9 and 5 (I couldn't test 6 because it segfaulted with an ICE on my simple test, I'll look into this separately).

So I think it would be reasonable to add

#if __GNUC__ == 4 && __GNUC_MINOR__ >= 4 && __GNUC_MINOR__ <= 7
#    pragma GCC diagnostic ignored "-Wparentheses"
#endif

to include/internal/catch_suppress_warnings.h. This would make CATCH usable for people forced to use these versions of the compiler (because currently it just isn't, until you add -Wno-parentheses to the compiler flags yourself).

[horenmar]

Interestingly, this is already suppressed but for C++11 only (can't tell you why)., because C++11 has _Pragma.

[vadz]

I understand the problem with suppressing the warnings in the entire TU, but IME this will end up being done anyhow when using one of the affected compiler versions because there are just too many useless warnings otherwise.

And doing it for C++11 doesn't seem to make that much sense because gcc < 4.8 didn't have full C++11 support anyhow, so the vast majority of people using C++11 with gcc wouldn't be affected by this warning even without this _Pragma.

[philsquared]

@horenmar did some more experimenting and found that _Pragma is supported back to at least GCC 4.6. He wasn't able to try 4.5, but 4.4 didn't work (if anyone else is able to confirm one way or another with 4.5 then please let us know).
So we're going to extend the use of _Pragma back to 4.6 (or possible 4.5), leaving only a couple of older versions in the not-covered range. If anyone else complains about these ones we'll consider the whole-TU-affecting option.

[horenmar]

@vadz The reason for using _Pragma is that in theory it lets us apply the pragma to single line, where Catch causes the warning, while letting the warnings be in rest of the TU. The reason why it is gated behind C++11 is that it was added to the language at that point and there was no testing done to see if earlier versions of GCC support it as well. Standard pragma is of no use, since we would have to let the suppression be enabled.

The good news is that I did some testing and gcc versions 4.6 and up support _Pragma.

The bad news is that there are some bugs in gcc's handling of _Pragma pre gcc-4.8, so it is useless anyway.

At this point we will probably end up just force disabling these for pre 4.8 gcc, and use the _Pragma option for post 4.8 gcc.

[vadz]

_Pragma was new in gcc 4.6 according to the docs.

Right now the compiler which troubles us is 4.5 (yes, don't laugh please) under some ancient CentOS that we need to support, so it won't really help here. So I still think we should use _Pragma if we can and disable the warning globally otherwise.

[obfuscated]

Seems fixed when using GCC 4.4. Thanks.

[mloskot]

#674 (comment)

-Wno-error=parenthesis is another way

Typo: parentheses