[llvm-commits] [llvm] r158638 - in /llvm/trunk: include/llvm/ADT/DenseMap.h unittests/ADT/DenseMapTest.cpp
Nick Lewycky
nicholas at mxc.ca
Mon Jun 18 21:12:02 PDT 2012
Chandler Carruth wrote:
> On Mon, Jun 18, 2012 at 8:37 AM, Duncan Sands <baldrick at free.fr
> <mailto:baldrick at free.fr>> wrote:
>
> Hi Chandler,
>
> > The codebase already has piles of these incorrect warnings in it,
>
> maybe clang does but LLVM proper does not, so why not keep it that way?
>
>
> Because the warning is broken. Fundamentally.
>
> There is no way to work around the warning that does not make the code
> worse. If the GCC optimizer cannot *prove* that the value is
> initialized, or does not hit one of its magical "don't warn on this"
> signals (usually escaped address to an external function), it calls it
> uninitialized.
>
> If you initialize it blindly, then any actual bug you might have in your
> code will fail to be reported by Valgrind and other tools that really
> can detect uninitialized uses.
>
> Now, maybe I've introduced a bug into this code and the warning is
> legitimate. I'll look into that. But I am completely opposed to changing
> code to be less readable, less accurate, less efficient, or less easy
> for Valgrind to check to satisfy a poorly implemented warning when we
> have better alternatives.
>
> We should rely on Clang's -Wuninitialized and on Valgrind to catch our bugs.
I agree with this in this case, but it hasn't escaped my attention that
LLVM has always had a "no warnings allowed" policy which we've applied
evenly across all situations (see
http://llvm.org/docs/DeveloperPolicy.html#quality ). Since I think this
is a legitimate problem, I want to discuss what the policy ought to be.
We should err heavily towards the side of respecting the compilers
warning, except when we know that the compiler warning is Plain Broken
(as it is here). We should have a list of warning+compiler pairs that
are known broken (perhaps next to the brokengcc list?).
The reason that the old policy doesn't work any more is that as we've
grown we've picked up more users using more distinct versions of more
compilers than ever before, and our probability of hitting a badly
implemented warning (even one that's just-a-bug from the compiler
vendor) is going up.
If folks agree with this, we should probably prepare a patch to the
DeveloperPolicy to update its text slightly, and start our list somewhere.
Nick
More information about the llvm-commits
mailing list