Disable -Wparentheses warnings for g++ < 4.8

[vadz]

Since the changes of #247, a "warning: suggest parentheses around comparison
in operand of '=='" is generated for every use of CHECK(x == y) under -Wall.

Unfortunately the only way to get rid of the flood of such warnings seem to be
to disable it completely. At least this only has to be done for g++ <= 4.7 as
4.8 and later don't give these warnings for the use of CHECK() any more.

[vadz]

Actually it looks like in more complicated cases even g++ 4.9 still warns about this. So the choice seems to be between

1. Always disabling -Wparentheses when using CATCH.
2. Putting pragmas to ignore and restore it inside all macros using Catch::ResultBuilder.
3. Reverting switch from ->* to <= in expression decomposer #247.

As much as I dislike this particular warning, I don't think (1) is acceptable, a testing library shouldn't dictate the choice of compilation options, I could live with it if this were only needed for the old compilers, but not if it's always the case. It looks like (2) should be doable, but that's a lot of work just to suppress a warning. So I really wonder if the benefits of the change in #247 were worth it and if (3) isn't the best thing to do.

The fact is that using the latest CATCH I get thousands of warnings in my tests and something needs to be done about it.

[rolanddenis]

Hello,

firstly, I want to warn you that this PR is a duplicate of a previous one, #521. Having said that, this problem bothers me too and I've already tried some of your propositions.

I totally agree with you that (1) is not a good idea. The solution (2) is not so complicated: in fact, the warning comes from the line https://github.com/philsquared/Catch/blob/master/include/internal/catch_capture.hpp#L36 in INTERNAL_CATCH_TEST macro. But, as we are in a macro, we cannot use common #pragma syntax to locally disable the warning. Instead, there is the _Pragma operator that do the same and we obtain something like this:

_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wparentheses\"") \
 ( __catchResult <= expr ).endExpression(); \
_Pragma("GCC diagnostic pop") \

On my side, I couldn't suppress all warnings with this modification and it seems to be related to multiple bugs with _Pragma that "delayed" his effect:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715271
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66099
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53469

In addition, this would imply to add such pragma for each compiler that warns about the parentheses (and I don't understand why they don't do it).

About the solution (3), I think it is a good thing to allow expressions like REQUIRE( 1+2 == 3) without extra parentheses. Nevertheless, @tavianator in #247 suggests to use << instead of <= (that has an intermediate precedence between comparison and addition). I've tried with >> (as << operator of ResultBuilder is already used) and no more warnings with g++.
However, I get many warnings from clang (-Woverloaded-shift-op-parentheses) ...

PS: how did you gets the warning with g++ 4.9 ?

[vadz]

@rolanddenis thanks, I didn't notice #521 somehow.

I did plan to try using _Pragma but if it doesn't work anyhow because of compiler bugs, I'm not sure if it's worth spending time on this.

The case of 4.9 is weird, I can't reproduce the warning in a simple example with 4.9.3 (from Debian), but I see it in the buildbot logs on a machine using 4.9.2. FWIW the warning can be reliably reproduced in simple example with all of 4.4, 4.5, 4.6 and 4.7, it's enough to do this:

echo 'int main() { int x = 2; return 1 <= x == 3; }' | g++ -Wall -c -fsyntax-only -x c++ -

[rolanddenis]

I took the test and what works, however, is to replace <= by >> and to neutralize clang warnings (-Woverloaded-shift-op-parentheses) with _Pragma (that seems less buggy than with g++). But I have no idea what other compilers say about that ...

I use the version 4.9.2 and it emits warnings on 1 <= x <= 3 and similar expressions but I didn't get any warnings with Catch.

[bdb]

I'm getting this too with v1.2.1, my solution with the single header was just to change line 1574:

•        ( __catchResult <= expr ).endExpression(); \
  

•        ( __catchResult <= (expr) ).endExpression(); \
  

This allows GCC 4.9.2 to compile our tests cleanly again. (We were using v1.0.53 before w/o issue).

[bdb]

Here's an example that causes 4.9.2 to emit the parens warning:

error: suggest parentheses around comparison in operand of ‘==’ [-Werror=parentheses]
CHECK(comps.at(1) == "Users");

comps is a vector of strings

[martinmoene]

@bdb the () around expr in __catchResult <= (expr) defeat the expression decomposition mechanism...

[bdb]

@martinmoene True, but so does adding explicit parens in your test CHECK() usage. I was not suggesting my patch as official solution merely noting an easy way to solve hundreds of warnings (errors with -Werror) in one go. As I said this was an issue for us upgrading from 1.0.53, it broke a lot of tests.

We've run in to parens issues before with v53, but they were minor and to solve them we did add explicit parens to the offending CHECK() statements.

[martinmoene]

@bdb ah, I wasn't aware of its temporarilyness.

[philsquared]

Apologies for the premature closure - I deleted the branch and it auto-closed all associated PRs - it wasn't intentional.
I also can't re-open it at this point - but I'll come back and review shortly.

[bdb]

Thanks Phil. Still same issue (and same workaround) in 1.3.0