Fix parentheses warning with Catch

[rolanddenis]

Following the discussion in #1060, this PR proposes a solution to avoid warnings about missing parentheses in Catch assertions when using gcc (up to 4.7 but following versions may be concerned too).

Until now, the solution was to add extra parentheses around expression like this:

int a = 1, b = 2;
REQUIRE(( a == b ));

As the assertion failed, Catch outputs:

test.cpp:123: FAILED:
  REQUIRE( ( a == b ) );
with expansion:
  false

but one of the main argument of Catch is his ability to expand expressions. For example, without extra parentheses:

int a = 1, b = 2;
REQUIRE( a == b );

Catch outputs:

test.cpp:123: FAILED:
  REQUIRE( a == b );
with expansion:
  1 == 2

This issue has been addressed many times in Catch project ( see catchorg/Catch2#528, catchorg/Catch2#521 and catchorg/Catch2#247 ) and the solutions proposed so far are:

1. to globally disable -Wparentheses, at least when using gcc,
2. to locally disable this warning in Catch,
3. to revert switch from ->* to <= in expression decomposer catchorg/Catch2#247,
4. to replace <= operator of ResultBuilder by an operator with a slightly greater priority, like << or >>.

The remarks about those solutions are:

1. it is quite ugly to globally disable warnings ... but, in fact, -Wall seems to be enable in DGtal only when BUILD_TESTING is set (see https://github.com/DGtal-team/DGtal/blob/master/cmake/CpackCtest.cmake ).
2. the problem comes from the line https://github.com/DGtal-team/DGtal/blob/master/tests/catch.hpp#L2073 . As we already are in a macro, we must use _Pragma to tell gcc to disable the warnings:

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

Unfortunately, there is many bugs with _Pragam and gcc that make it useless here.
3. Reverting catchorg/Catch2#247 implies that expressions like REQUIRE( 1+2 == 3 ) do not compile anymore (extra parentheses are then needed around 1+2).
4. Using >> instead of <= removes warnings with gcc but clang wakes up with -Woverloaded-shift-op-parentheses warning ...
Fortunately, _Pragma works fine with clang and it enables us to clean the warns.

This pull request is about that last solution. I don't know if other compilers (clang on Mac, icc, visual studio, ...) are ok with that PR.

[rolanddenis]

One more note: the problem here is that we are deviating from the Catch master (in addition to the template test cases) and I don't know what solution will be chosen in Catch.

[dcoeurjo]

Thanks a lot for this PR (and the very informative description).
Let's merge this one wait for Catch decision. In order to keep track of that in DGtal, could you please add an entry to the Chanelog ?

[rolanddenis]

Done ;)

[dcoeurjo]

Perfect, thanks.

[rolanddenis]

Huh ... if not too late, could you reopen this PR ? I have some warnings back :/
I will also add some checks in the test file.

[dcoeurjo]

mmm... let's try a revert..

[dcoeurjo]

ok crap.
you'd need to reopen the RO :)

[rolanddenis]

Thanks but I don't know what to do since
REQUIRE( 1+2 == 3 )
now emits a warning about missing parentheses around 1+2, even with gcc 4.9.2. (I was pretty sure to have tested this ...)

Finally, maybe the best choice is

• to revert switch from ->* to <= in expression decomposer catchorg/Catch2#247 and go back to -> operator (or to keep the current PR with >>), while adding extra parentheses around left term in each comparison,
• or to do nothing and wait for an official fix.

[dcoeurjo]

Can you please reopen the PR? I'll test what's happening on my os.

[rolanddenis]

Done: #1069