[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

[Nico Weber]

Thanks for the quick reply.

On Wed, Jan 26, 2011 at 5:38 PM, Ted Kremenek <kremenek at apple.com> wrote:
> On Jan 26, 2011, at 5:17 PM, Nico Weber wrote:
>
> Hi Ted,
>
> this adds 265 compile errors to the chrome build (we build with
> -Werror). I went through the first 40 or so, and they're all false
> positives. The two most common cases are
>
> Hi Nico,
> Thanks for filing these reports.  I expected some fallout from enabling this
> warning, and I want to see these bugs.
> One thing I want to make clear is that the goal of -Wuninitialized in clang
> is twofold:
> 1) Provide consistent results despite the optimization level used for
> compilation (or even -O0)
> 2) Provide reasonable but conservative results.  I suspect that GCC gives up
> in many cases.
>
> http://llvm.org/bugs/show_bug.cgi?id=9063
>
> I think there are two cases here, and I've commented in the bug requesting
> more data.

Replied on the bug.

>
> http://llvm.org/bugs/show_bug.cgi?id=9064 (a real bug)
>
> Yes this is a glaring omission.  Just need to handle fast enumeration.
>
> I also filed
> http://llvm.org/bugs/show_bug.cgi?id=9061
>
> I think this is reasonable to handle, but only for the case where loops have
> the format:
>   for (int i = 0; i < constant ; ...)
> It's clear that the loop body is always entered.

This would help, I've seen this pattern (for loop with a fixed upper
bound) a few times already.

> http://llvm.org/bugs/show_bug.cgi?id=9062
>
> I don't think this is reasonable to do.  I suspect GCC just gets lucky
> because it's -Wuninitialized implementation in based on the optimizer or
> that it simply gives up.  There is a control-dependency here that in the
> general case can only be handled using path-sensitive analysis.  Going back
> to goals (1) and (2), I think this is one place where we want Clang to be
> more conservative than GCC.  I suspect many will disagree.

I've seen this problem only once so far, might not be a big deal.

>
> At the moment, we will have to disable -Wuninitialized in chrome if we
> want to update clang (which we _always_ want to do :-) ).
>
> I think 3/4 PRs are reasonable requests to fix.  The only contentious one is
> 9062, and I don't know if that is a deal breaker for you.

I don't know yet, it depends on how the false positive rate looks
after the new engine had some baking time. Thanks for being responsive
:-)