r224329 - Renamed RefersToEnclosingLocal bitfield to RefersToCapturedVariable.

Richard Smith richard at metafoo.co.uk
Sun Jan 4 01:11:07 PST 2015


On Sat, Jan 3, 2015 at 1:46 PM, John McCall <rjmccall at apple.com> wrote:

> On Jan 2, 2015, at 7:10 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Tue, Dec 16, 2014 at 12:01 AM, Alexey Bataev <a.bataev at hotmail.com>
> wrote:
>
>> Author: abataev
>> Date: Tue Dec 16 02:01:48 2014
>> New Revision: 224329
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224329&view=rev
>> Log:
>> Renamed RefersToEnclosingLocal bitfield to RefersToCapturedVariable.
>> Bitfield RefersToEnclosingLocal of Stmt::DeclRefExprBitfields renamed to
>> RefersToCapturedVariable to reflect latest changes introduced in commit
>> 224323. Also renamed method Expr::refersToEnclosingLocal() to
>> Expr::refersToCapturedVariable() and comments for constant arguments.
>> No functional changes.
>>
>
> This seems like a bad idea for me. It's incorrect: for a lambda, the flag
> means that the DeclRefExpr refers to an enclosing local, and does *not*
> imply that the variable is necessarily captured. This confusion has already
> led to a bug (fixed in r225060).
>
>
> If that’s actually a useful property to track, I have no complaint about
> tracking the two things separately.  I don’t think DRE is short of bits.
>

The problem is that we can't reasonably track whether a DRE refers to a
captured variable, because that is not a local property of the DRE; it
depends on non-trivial properties of the context in which the DRE is used
(and more generally it depends on how else that same variable is used
elsewhere in the lambda, but we can largely ignore that).

I'd be fine having separate flags for 'refers to enclosing local' and
'refers to captured global' (if the latter is the point of this change),
but we should not claim a DRE refers to a capture when it does not.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150104/e27e667b/attachment.html>


More information about the cfe-commits mailing list