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

[David Blaikie]

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)


>
> > 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).

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.

- 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/311babf0/attachment.html>