On 2019-10-22 22:25, Kim Barrett wrote:
On Oct 22, 2019, at 3:17 AM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 
wrote:

I have tested that this compiles without warnings on gcc 4.8, 5.5, 6.5, 7.4 and 
8.3 on x64. I have also tried building zero on x64, aarch64 and arm32 with gcc 
8.3.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211073
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8211073-enable-extra-on-hotspot/webrev.01
Change looks good.
Thanks.

I remain somewhat leary of omnibus warning options in a code base like
this, which is large and needs to support multiple compiler versions
on multiple platforms. But at least most can be disabled if needed.
It's a three-edged sword.

I like the concept of saying to the compiler "Here's my code, give it your best shot to find issues with it", and then explicitly telling it "well, no, that's not really a problem, ignore it". And if we really had to specify *each and every* warnings that a compiler should check for, the command lines would be excruciatingly long.

On the other hand, if the set of warnings change between compiler versions, we have a harder time keeping the code warning free. Once again, if the warnings are reasonable, fixing issues is a good thing (and they could even have been avoided by writing good code in the first place :-)). But sometimes they are over-zealous and it is just an annoyance to fix.

But then again, even for an individual, specified warning, the conditions for when and how it applies can change between compiler versions, so the same situation is really always present, even if it's on a "less likely" scale.


Note that gcc9 adds:
-Wdeprecated-copy
   Warn that the implicit declaration of a copy constructor or copy
   assignment operator is deprecated if the class has a user-provided
   copy constructor or copy assignment operator, in C++11 and up. This
   warning is enabled by -Wextra. With -Wdeprecated-copy-dtor, also
   deprecate if the class has a user-provided destructor.

-Wredundant-move
   This warning warns about redundant calls to std::move; that is, when a
   move operation would have been performed even without the std::move
   call.

Neither of these are an issue for us yet, because we're not yet using
C++11 or later.  But I expect -Wdeprecated-copy to cause problems for
that update, and we might need to (temporarily) globally disable it as
part of JEP 347.

It's annoying that some of its warnings have no associated -Wno-xxx,
in particular the one about mixing integral types and enum types in
conditional expressions, because of the history of our code base and
because it's perfectly well-formed usage. But apparently we don't have
any of those right now.

On Oct 22, 2019, at 4:28 AM, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 
wrote:
I wouldn't count on gcc 9 bringing in anything new to -Wextra. In general, gcc 
is *extremely* conservative about changing -Wextra nowadays. Virtually all new 
warnings that are added are added as indivudual warnings with explicit names to 
be turned on or turned off. Only after a long vetting process does any of these 
gets added to -Wextra. I think the last time anything was added to -Wextra was 
in like gcc 6..? So the -Wextra is in a way a bit of a legacy system in gcc for 
handling warnings.
This depends on what you mean by additions or changes to -Wextra. As
mentioned above, gcc9 adds two new warnings, but they can be
individually controlled. The set of warnings only provided by -Wextra
and not individually controlled has indeed not changed in a long time.
I checked, and gcc4.9 and gcc9.2 have the same set of those.

Yes, new warnings can be individually disabled. It's a shame that not all -Wextra warnings can be, but unless we're prepared to submit a patch to gcc, I assume we're in no position to complain. :-)

/Magnus

Reply via email to