[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