[cfe-dev] Lambda expr AST representation

[Abramo Bagnara]

Il 09/10/2012 17:38, Douglas Gregor ha scritto:
> 
> On Oct 9, 2012, at 8:11 AM, Abramo Bagnara
> <abramo.bagnara at bugseng.com> wrote:
> 
>> Il 09/10/2012 15:29, Douglas Gregor ha scritto:
>>> 
>>> On Oct 4, 2012, at 2:36 PM, Abramo Bagnara 
>>> <abramo.bagnara at bugseng.com> wrote:
>>> 
>>>>>> I'd suggest a slightly different path:
>>>>>> 
>>>>>> 1) the closure type FieldDecl has the name (actually a 
>>>>>> pseudo-name) of the captured variable ("this" to refer to 
>>>>>> captured this)
>>>>>> 
>>>>>> 2) the FieldDecl uses a bit to represent the fact that
>>>>>> fields are the fields of the closure type (this means they
>>>>>> are actually unnamed)
>>>>>> 
>>>>>> In this way the source pretty printing is easily doable,
>>>>>> the semantic info is accurate, no new AST node is needed,
>>>>>> CodeGen is simpler (it does not need to map DeclRefExpr to 
>>>>>> MemberExpr).
>>>>>> 
>>>>>> I've forgot something?
>>>>> 
>>>>> That could work... although it would be a bit tricky to find
>>>>> the original captured variable given a MemberExpr of this
>>>>> sort.
>>>> 
>>>> I've thought to that, but I failed to imagine a case where this
>>>> is needed.
>>> 
>>> It matters a lot for features that care more about the results
>>> of name lookup than the underlying semantics. For example,
>>> libclang's clang_findReferencesInFile, which finds all of the
>>> references to a given declaration, would need to introduce new
>>> code to map the fields of implicitly-generated MemberExprs back
>>> to references to a normal variable declaration. In general, these
>>> tools expect (reasonably, IMO) that a local variable or static
>>> data member will be referenced with DeclRefExpr, while a
>>> non-static data member will be referenced with a MemberExpr.
>>> That's actually a very nice invariant. Doing as you suggest would
>>> complicate the invariants for these clients, forcing them to deal
>>> specifically with lambda captures (which they otherwise wouldn't
>>> have to consider). And if we have to have the complication
>>> somewhere, I'd rather it be with the more intelligent clients
>>> that care about semantics, rather than the clients that only care
>>> about cross-referencing.
>> 
>> I have a rather different perspective for libclang that IMHO is
>> more accurate (and congruent with semantic): the variable in body
>> references the capture list entry (implicit or explicit) while the
>> capture list entry references the captured variable.
> 
> You're saying it's "more accurate" because it matches more closely
> with the as-i" implementation written in the standard, and I don't
> dispute that. What I dispute is that exactly modeling the as-if rule
> in the standard is the right thing for Clang. We're not bound to
> implement lambda classes via exactly that as-i" rule, and we probably
> shouldn't do so for [&] lambdas anyway (because it's silly to store
> several references to local variables in the same stack frame). And,
> as noted above, our ASTs are meant to describe the language (which
> they certainly do, even if not strictly based on the as-if rule) and
> are meant to be usable by clients. Your suggestion may make some
> trivial improvement in the former (for those who want to think in
> terms of the as-if rule), but complicate other clients. That's not a
> good trade-off.

I'm rather unconvinced this is a good choice: what about AST Matchers
that want to find references to captured variable? I'm sure there are
also other example of things that become rather complex if we permit
such a large deviation from correct semantics to AST representation (and
as far as I know this is almost unprecedented).

>>>> 1) the closure type FieldDecl has the name (actually a
>>>> pseudo-name) of the captured variable ("this" to refer to
>>>> captured this)
>>> 
>>> As noted elsewhere, we can't do this exactly. The __name bit
>>> could work, but I'd prefer that we simply keep these as anonymous
>>> fields, because the __ names really don't help that much.
>> 
>> I apologize, it seems I've missed the elsewhere where this has
>> been clarified. Can you explain that?
> 
> The __ names still require you to detect that you're in a lambda and
> then remove the '__' from the name. You can't simply remove __ from
> every name, or you'll get bogus results from various system classes
> that actually do use __.

I meant to name the fields exactly with the captured name (without __
prefix) and "this" for captured this. These names would be hidden to lookup.

>> It would be a pity to not have such names in FieldDecl, this would
>> make pretty printer job very easy.
> 
> Adding a
> 
> VarDecl *getCapturedVariable(FieldDecl *)
> 
> for lambda classes would make the pretty printer's job trivial.
> Besides, what's a pretty-printer doing printing the generated lambda
> class?

The body of operator() of lambda class should be printed.

-- 
Abramo Bagnara

BUGSENG srl - http://bugseng.com
mailto:abramo.bagnara at bugseng.com