[PATCH] D30327: [Sema] Improve side effect checking for unused-lambda-capture warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!

In https://reviews.llvm.org/D30327#688254, @malcolm.parsons wrote:

> I found this FIXME comment in `Expr::HasSideEffects()`:
>
>   case LambdaExprClass: {
>     const LambdaExpr *LE = cast<LambdaExpr>(this);
>     for (LambdaExpr::capture_iterator I = LE->capture_begin(),
>                                       E = LE->capture_end(); I != E; ++I)
>       if (I->getCaptureKind() == LCK_ByCopy)
>         // FIXME: Only has a side-effect if the variable is volatile or if
>         // the copy would invoke a non-trivial copy constructor.
>         return true;
>     return false;
>   }
>   
>
> It seems a shame not to fix this now, but I don't think I can call Sema code from AST code, and my `CaptureHasSideEffects()` depends on `Sema::getCurrentThisType()`.
>
> Any ideas?


Correct, you cannot call Sema code from AST. I don't know of a good way to resolve this off the top of my head given what `getCurrentThisType()` needs to do. I agree that this would be good to fix as well, but your patch is a good incremental improvement and should't be held up on this.


https://reviews.llvm.org/D30327