On Tue, Apr 28, 2015 at 10:38 AM, Aaron Ballman <[email protected]> wrote:
> On Tue, Apr 28, 2015 at 1:23 PM, David Blaikie <[email protected]> wrote: > > > > > > On Tue, Apr 28, 2015 at 10:11 AM, Aaron Ballman <[email protected]> > > wrote: > >> > >> On Tue, Apr 28, 2015 at 12:22 PM, David Blaikie <[email protected]> > >> wrote: > >> > On Tue, Apr 28, 2015 at 5:36 AM, Aaron Ballman < > [email protected]> > >> > 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 > >> >> [email protected] > >> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
