boostorg / mpl

Boost.org mpl module
http://boost.org/libs/mpl
Boost Software License 1.0
52 stars 78 forks source link

Unnecessary parenthesis warning in gcc-8.0.0 powerpc matrix build #31

Closed jeking3 closed 6 years ago

jeking3 commented 6 years ago

See:

http://www.boost.org/development/tests/develop/output/trippels-powerpc64le-gcc-8-0-date_time-gcc-8-0-0-warnings.html

Warnings: trippels-powerpc64le-gcc-8.0 - date_time / gcc-8.0.0
Rev c1a20921c54ad6354c3c93e532e76c62b5c598a9 / Sat, 06 Jan 2018 13:35:45 +0000

...

testc_local_adjustor
../boost/mpl/assert.hpp:188:21: warning: unnecessary parentheses in declaration of ???assert_arg??? [-Wparentheses]
../boost/mpl/assert.hpp:193:21: warning: unnecessary parentheses in declaration of ???assert_not_arg??? [-Wparentheses]

testclock
../boost/mpl/assert.hpp:188:21: warning: unnecessary parentheses in declaration of ???assert_arg??? [-Wparentheses]
../boost/mpl/assert.hpp:193:21: warning: unnecessary parentheses in declaration of ???assert_not_arg??? [-Wparentheses]

... (many more)
swatanabe commented 6 years ago

AMDG

This warning is ridiculous. What harm do unnecessary parentheses do? It doesn't make the code less readable and it's not going to make the code mean something that might not be intended either.

In Christ, Steven Watanabe

eldiener commented 6 years ago

I agree with Steven. Attempts to satisfy syntactical compler warnings by changing completely normal C++ code are a huge waste of time, and there is no end to it. This is why I opposed the vc++ shadow warnings fix earlier in date_time and this is yet another example of syntactical warnings that I personally have no intention of wanting to change. if others want to address them, go ahead, but I will not be doing so in any way.

jeking3 commented 6 years ago

It only shows up in the gcc-8 matrix test. Don't shoot the messenger. Close it if you want.

eldiener commented 6 years ago

I did not mean to sound harsh. Sorry about that. I will let any others chime in on this before I close it.

jeking3 commented 6 years ago

It's okay, I'll close it. :)

Romain-Geissler-1A commented 6 years ago

Hi,

I created PR #33 before seeing this issue. So obviously you will not accept it. I have no strong opinion on this warning, however I expect that a Boost user can decide to build with -Wall -Wextra without having warnings from Boost. So would you accept a pull request that explicitly ignore these kind of warnings in assert.hpp using the usual Boost mechanism to ignore expected warnings ?

Cheers, Romain

mclow commented 6 years ago

Romain -- My attitude towards warnings has changed over time; I used to believe as you do, that we should deal with all the warnings. However, compilers have added more and more warnings over time, and while most of them are useful, some of them are not. [ Examples: MSVC's 'truncation' warning, Clang's (now neutered) 'tautological limits' warning, and GCC's 'extra parens' warning. ] Titus Winters said it well in his talk about Abseil at https://www.youtube.com/watch?v=xu7q8dGvuwk, where he says that "some warnings can DIAF"

Couple that with more and more people building with -Wall -Wextra -Werror it means that Boost has to be 'ahead of the curve', and find new warnings in unreleased compilers and either (a) "fix" the boost code, (b) suppress the warning, or (c) convince the compiler writers that their warnings are wrong (frequently more than one of the above)

I expect that "surpassing the warning" is the best solution here.

Romain-Geissler-1A commented 6 years ago

Just for me to fully understand, by "surpassing the warning" you mean (b) or (c) (or both ?).

About warning that do not always make sense, well look at the different gcc bug reports I have created for new ones around strncpy. These depends on how good gcc is at inlining things and how you write your code to make the compiler understand you actually want your code like this. Example: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84468 I also agree that sometimes warning should be challenged, however in most of the cases I have seen so far, they did uncover real bugs (and despite I have had many troubles with the new gcc 8 strncpy warnings, I discovered few but still some real bugs in my code).

About having boost being ahead the curve with unreleased compiler, well you have your own CI on your side, + all the people like me who are actually paid by companies to migrate to newer compiler, newer boost, newer everything, and who try to fix these issues when they see them. Boost is normally big enough to have people contributing to either fix the warnings, or silence them.

Romain-Geissler-1A commented 6 years ago

Hi,

I have submitted #34 which does the same thing as #33 but this time by ignoring the warning.

Cheers, Romain

ningfc commented 4 years ago

I have build boost with gcc 7.3.0, that is OK