On Fri, 1 Feb 2019, Jeff Law wrote:
On 2/1/19 7:01 AM, Marek Polacek wrote:
On Fri, Feb 01, 2019 at 07:19:25AM -0600, Segher Boessenkool wrote:
Hi Marc,
On Fri, Feb 01, 2019 at 12:32:45PM +0100, Marc Glisse wrote:
-Wmaybe-uninitialized generates false positives, we can tweak the compiler
to reduce them, but there will always be some, that's in the nature of
this warning.
That is true for *every* warning; if not, it should be an error, not a
warning.
My opinion is that -Wmaybe-uninitialized would serve its purpose better as
part of -Wextra.
+1
+1 from me too.
I disagree strongly.
I am not surprised, but I had to at least start the conversation. Would
you mind providing a patch that changes the definition of -Wall, since the
current one doesn't quite match reality anymore? Also, what do you
recommend people do when they hit false positives?
If we move it to Wextra it's going to see a lot less usage in real world
codebases
I am very tempted by the strawman: should we deprecate -Wextra since
nobody uses it? (Sorry)
Ideally serious projects would use (parts of) -Wextra, at least
occasionally, and with circumspection. But some warnings like
-Wmaybe-uninitialized are dangerous tools in the hands of quickfix
developers, and I am indeed trying to keep it out of their hands...
and potentially lead to the re-introduction of a class of bugs that
we've largely helped stomp out.
That's very optimistic. -Wmaybe-uninitialized only detects a very small
proportion of uninitialized uses. Also, my experience is that people have
stomped out the warning, not the bugs. In some cases they even introduced
bugs to stomp out false warnings, or made it harder to detect real bugs in
the future, so the warning did more harm than good. I am mostly working on
large C++ header-only template-heavy scientific libraries, it is quite
possible that people who handle different types of code bases have a
different experience, and -Wmaybe-uninitialized may have had a more
positive impact on other projects.
It's also the case that maybe uninitialized vs is uninitialized is
really just a function of CFG shape. Give me any "maybe uninitialized"
case and I can turn it into a "is uninitialized" with simple block
duplication of the forms done by jump threading, path isolation,
superblock formation, etc.
Hmm, you know those things better than me, so you are probably right, but
I am not seeing it. We omit "maybe" if, starting from the entry of the
function, and barring exceptions, we know the statement will always be
executed. If you take a false positive for maybe-uninitialized, i.e. a
statement in a dead branch, I don't see how block duplication can make it
so the statement is now always executed. The natural way to remove "maybe"
is through function cloning or outlining. Then you can create functions
that are never called, and any warning about those is a false positive.
There is a matter of statistics. In practice maybe-uninitialized has way
more false positives than uninitialized, which makes it more problematic.
But if you prefer to move both to -Wextra (this is the current default
when front-ends don't override it), that's ok with me ;-)
--
Marc Glisse
