[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

John McCall via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 17 14:03:19 PDT 2016


rjmccall added a comment.

In https://reviews.llvm.org/D25556#572054, @ahatanak wrote:

> In https://reviews.llvm.org/D25556#569809, @rjmccall wrote:
>
> > Richard should probably weigh in about whether we should be recording potential captures at *all* capturing scopes.  But at the very least, I think you have a bug here where the variable is declared outside of the block but within the lambda; in that case, it is definitely not a potential capture of the lambda.
>
>
> Ah, that's a good point.
>
> When a variable is declared outside of the block but within the lambda, CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures in SemaExprCXX.cpp will not capture the variable because getStackIndexOfNearestEnclosingCaptureCapableLambda doesn't return a proper index, so it looks like it's still safe to add the variable here even when it cannot be captured by the lambda. I think it's possible to check that the variable is not declared within the lambda before adding it to the list of potential captures in the code above, but I'm not sure whether we have to do that.
>
> Also, could you elaborate on when you think we don't have to record a variable as a potential capture? My understanding is that you need to record the variable as a potential capture when the variable is part of a full expression that depends on a generic parameter because you don't know whether the variable is odr-used and needs to be captured by the lambda until you look at the full expression. Perhaps there are exceptions to this rule when the variable is used in a block?


I mean that it might be important for us to be recording this at block scopes, not just lambda scopes.  Maybe not; I guess we do eagerly instantiate blocks, which we can't necessarily do with lambdas.  This is something that Richard needs to weigh in on.


https://reviews.llvm.org/D25556





More information about the cfe-commits mailing list