r235981 - Silencing a spurious -Wuninitialized warning with this local; NFC.

[David Blaikie]

On Tue, Apr 28, 2015 at 10:38 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Tue, Apr 28, 2015 at 1:23 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Tue, Apr 28, 2015 at 10:11 AM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> On Tue, Apr 28, 2015 at 12:22 PM, David Blaikie <dblaikie at gmail.com>
> >> wrote:
> >> > On Tue, Apr 28, 2015 at 5:36 AM, Aaron Ballman <
> aaron at aaronballman.com>
> >> > wrote:
> >> >> Author: aaronballman
> >> >> Date: Tue Apr 28 07:36:54 2015
> >> >> New Revision: 235981
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=235981&view=rev
> >> >> Log:
> >> >> Silencing a spurious -Wuninitialized warning with this local; NFC.
> >> >
> >> > Which compiler warned on this?
> >>
> >> I believe it's gcc, but I do not have the version information handy.
> >> It's the bot responsible for building our attribute documentation,
> >> which I keep warning-free. (Tanya would probably be able to get that
> >> information for us if it was important.)
> >
> >
> > Can sometimes make a reasonable guess based on the diagnostic style/text
> -
> > though GCC 5-ish has started doing the quoted samples like Clang (though
> it
> > doesn't highlight ranges, so that's still a distinguishing feature even
> in
> > non-colored output)
>
> Yeah, that's why I was guessing GCC. Doesn't look like Clang output to me.
>
> >
> >>
> >>
> >> > It looks like this initialization is not needed - the switch over
> >> > ImpCaptureStyle (speaking of which, the declaration of this variable
> >> > could be moved down to closer to this switch - it isn't used before
> >> > the switch) seems to initialize the variable on all paths that are
> >> > reachable.
> >>
> >> Correct, that's why I called it a spurious warning. ;-)
> >
> >
> > Ah, sorry, missed that.
> >
> >>
> >>
> >> > (the usual "excessive initialization hampers checkers like msan and
> >> > doesn't make the code better because the default value is never
> >> > intended to be used - so the program's already off the rails if it's
> >> > used")
> >>
> >> Also agreed. If you have a better idea of how to silence the warning
> >> so that build remains warning-free, I would welcome suggestions.
> >
> >
> > Disable bad warnings - we've certainly disabled a few of GCC's for just
> this
> > reason before, I'm surprised this variation on the theme has survived
> this
> > long, honestly (I guess -Wmaybe-uninitialized was the biggest culprit).
>
> I don't think this warning is a reasonable candidate to disable based
> on how infrequently we run into FPs with it.
>

My tolerance is fairly low because it seems it doesn't buy us much, so even
a small false positive count seems enough to disable it when we'll catch
the good cases with clang anyway. (& it means I don't have to worry about
missing a code review motivated by the warning)


> > If we really want to avoid that, you could pull out the switch into a
> helper
> > function that you call to retrieve the desired value. The
> llvm_unreachable
> > will stop GCC from warning about -Wreturn-type and the variable
> initialized
> > with the result of the call will be always initialized. (& then different
> > sanitizers would catch the sort of bugs that would lead to the
> unreachable
> > or otherwise exiting the function without a return value) - also makes
> the
> > code easier to read/reason about, since you don't have to scan the
> switch to
> > check that the variable's initialized.
>
> This seems reasonable. I've implemented it in r236002. Thank you for
> the suggestion!
>

cool cool :)

- David


>
> ~Aaron
>
> >
> > - David
> >
> >>
> >>
> >> ~Aaron
> >>
> >> >
> >> >>
> >> >> Modified:
> >> >>     cfe/trunk/lib/Sema/SemaLambda.cpp
> >> >>
> >> >> Modified: cfe/trunk/lib/Sema/SemaLambda.cpp
> >> >> URL:
> >> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=235981&r1=235980&r2=235981&view=diff
> >> >>
> >> >>
> ==============================================================================
> >> >> --- cfe/trunk/lib/Sema/SemaLambda.cpp (original)
> >> >> +++ cfe/trunk/lib/Sema/SemaLambda.cpp Tue Apr 28 07:36:54 2015
> >> >> @@ -1482,7 +1482,7 @@ ExprResult Sema::BuildLambdaExpr(SourceL
> >> >>    // Collect information from the lambda scope.
> >> >>    SmallVector<LambdaCapture, 4> Captures;
> >> >>    SmallVector<Expr *, 4> CaptureInits;
> >> >> -  LambdaCaptureDefault CaptureDefault;
> >> >> +  LambdaCaptureDefault CaptureDefault = LCD_None;
> >> >>    SourceLocation CaptureDefaultLoc;
> >> >>    CXXRecordDecl *Class;
> >> >>    CXXMethodDecl *CallOperator;
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> cfe-commits mailing list
> >> >> cfe-commits at cs.uiuc.edu
> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150428/f1f4d33b/attachment.html>