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
