[cfe-commits] r124279 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td test/Preprocessor/pragma_diagnostic_sections.cpp test/Sema/uninit-variables.c test/SemaCXX/uninit-variables.cpp

[Ted Kremenek]

On Jan 31, 2011, at 3:43 AM, Chandler Carruth wrote:

> Ted, much like Nico, I'm not really happy with this change.
> 
> 1) This is a major change in policy about -Wuninitialized that was not
> discussed previously on cfe-dev. Can we have a discussion there?
> Specifically, you cite goals for the flag that I can't find either
> documented or discussed anywhere. I'm not saying I disagree with your
> statements, just that it comes a bit out of the blue.

Hi Chandler,

I completely agree we should have a discussion.

> 
> 2) I think that folding all of these warnings under one flag is very
> problematic. At Google, we've struggled greatly to find even a
> somewhat reasonable way to warn about uninitialized variables, and
> while I'm not fond of the GCC model, I'm not very fond of this one
> either.

I think fine grain control over uninitialized variable warnings make sense, but I still think it makes sense that people have the ability to easily enable all uninitialized variable checking that is available.

> The current state makes it impossible for us to turn on
> -Wuninitialized (I'm having trouble even *counting* the times it fires
> in our codebase because every build dies so early), which is a serious
> regression. Previously, we had this flag on and it caught real bugs
> with zero false positives. Now those bugs will slip through.

I'd appreciate you filing specific bugs on those false positives.

> 
> I'd propose first moving this new functionality back under a separate
> flag, but perhaps one less scary than -Wuninitialized-experimental
> (maybe -Wmaybe-uninitialized? That flag was added to GCC recently
> based on our experience IIRC...), to avoid regression for large
> codebases currently using Clang. Then let's discuss in detail exactly
> how to balance the tradeoffs between false positives and reasonable
> analyses on the main mailing list, and how to break down these types
> of features. I don't want to dig into that here, as I think a wider
> audience would be better.
> 
> Just to be clear, I'm not at all opposed to this type of
> functionality, but I'm a bit hesitant to turn it on w/o thinking
> carefully about it, and finding good ways to control the various
> different categories of warnings so that different codebases with
> different tolerances for these warnings can get the most out of Clang.
> I'm also somewhat allergic to regressions. ;]

I am completely open to changing the flag, but as Nico pointed out the point of turning it on this way was to find these regressions and to measure people's expectations.  This is TOT, not a shipping version of Clang.  We frequently turn on warnings and features, see how they work, and reevaluate.  Only a limited set of people would have provided feedback on the warning if I had not done this.  It was also hard to gauge the potential blowback, and without actually measuring that it would have been hard to have a well-informed discussion on this feature.

I'll be honest that I'm concerned that we can make -Wuninitialized work in Clang with the same expectations that people have with GCC's implementation.  Since GCC's implementation depends on its backend optimizer, it gets a whole bunch of "smarts" for free when it comes to control-dependencies (although I suspect in many cases GCC just gives up, and conservatively not warn).  The static analyzer can easily reason about control-dependencies, but that inherently makes the dataflow analysis much more expensive (and non-linear).  Replicating some subset of that functionality in -Wuninitialized is not going to be cheap.