[cfe-dev] Need Feedback on this very preliminary code that implements a portion of generic lambdas.

[Richard Smith]

On Tue, May 14, 2013 at 9:48 PM, Faisal Vali <faisalv at gmail.com> wrote:

>
>
> On Tue, May 14, 2013 at 1:35 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Mon, May 13, 2013 at 10:25 PM, Faisal Vali <faisalv at gmail.com> wrote:
>>
>>> First off, this patch is far from ready for committing - it does not
>>> include
>>> enough tests - includes commented out code - and even some dead
>>> code. I shall try to address all those issues in the future - but for now
>>> I just wanted to put this out there for some early feedback (before I go
>>>  any deeper down a rabbit hole I need not have crawled into)
>>>
>>
>> Just a couple of thoughts below, I've not had time to study the patch
>> itself yet.
>>
>>
>>> The patch implements the following (an incomplete list):
>>>   - converting auto parameters to template parameters and creating a
>>>     function call operator template
>>>     - It does this by replacing the typesourceinfo of the parameter -
>>>       Should I avoid doing this? Is there a way to transform the auto
>>>       into a template type parameter, so that it subsequently behaves
>>>       as a template type parameter, but remembers that it was
>>>       once an auto (for error messages only, right?)
>>>
>>
>> Perhaps an AutoType whose deduced type is the template parameter would
>> work? However, note this comment from SubstituteAutoTransform:
>>
>>       // If we're building the type pattern to deduce against, don't wrap
>> the
>>       // substituted type in an AutoType. Certain template deduction rules
>>       // apply only when a template type parameter appears directly (and
>> not if
>>       // the parameter is found through desugaring). For instance:
>>       //   auto &&lref = lvalue;
>>       // must transform into "rvalue reference to T" not "rvalue
>> reference to
>>       // auto type deduced as T" in order for [temp.deduct.call]p3 to
>> apply.
>>
>> If we wanted that to work, we'd need to teach some parts of template
>> deduction to walk over the AutoType node.
>>
>> How significant is the diagnostic impact of performing the substitution?
>> Where, and how often, does it show up?
>>
>
>
> It shows up if deduction fails - for e.g:
>
> auto L = [](auto *a) -> void {  };
> L(1);
>
> f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:77:3:
> error: no matching function for call to object of type '<lambda at
>
> f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12>'
>   L(1);
>   ^
> f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12:
> note: candidate template ignored: could not match 'type-parameter-0-0 *'
> against 'int'
>   auto L = [](auto *a) -> void {  };
>
>
> I could live with that - but if we didn't want to - asides from
> considering the approach
> you presented,  what are your thoughts about the following approach :
> when emitting the error, check to see if it is a parameter of a generic
> lambda
> and sneak the auto back into the diagnositc - pros and cons?
>

I don't think we have enough context to work that out in the general case;
the simplest way of encoding such context would be to wrap the template
parameter in a type sugar node, which brings us back to the other approach.


>
>
>>
>>
>>>   - it makes TransformLambdaExpr and InstantiateFunctionDefinition
>>>     generic lambda aware (to allow for transformation of nested lambdas)
>>>     so that the
>>>   - Regarding capturing for generic lambdas:
>>>     1) A generic lambda (nested or not) will only have its captures
>>>        computed/finalized when its enclosing context is non-dependent
>>>        (and if an empty capture list, then no captures).
>>>     2) When a generic lambda is invoked (i.e a specialization is
>>> instantiated)
>>>        any variable from an outer scope that is odr-used, but not
>>> already
>>>        captured by the lambda expression, results in an arror.
>>>     3) If an outer lambda that is enclosed in a non-dependent context,
>>>        can't tell whether an inner lambda (with an implicit capture)
>>>        or a tree of inner lambdas (all with implicit captures)
>>>        might need to capture the variable for some
>>>        instantiation of that inner lambda, the outer lambda will
>>>        capture that variable (if it has a capture default).
>>>          - this is implemented by hooking into ActOnCallExpr
>>>            (is it enough to assume that the only cases where an inner
>>>              lambda might potentially need to capture a variable is
>>>              if it is used in a dependent function call (ctor, operator)
>>>              of the inner lambda, in a non-unevaluated context?)
>>>
>>
>> Per the proposal, you need captures in other situations:
>>
>> struct S {
>>   constexpr S() {}
>>   S(const S&) { puts("captured"); }
>>  };
>> void f() {
>>   constexpr S s {};
>>   [=] (auto x) {
>>     // captures 's' (even though no instantiation can need it),
>>     // because the full-expression depends on 'x'. Therefore
>>     // we must print "captured".
>>     (void)x, s;
>>   };
>> }
>>
>>           - Within ActOnCallExpr, I dig into each argument, and as
>>>            long as they are not in an unevaluated context (currently
>>>            I only check for sizeof), and are from a scope outside
>>>            the outer lambda, I capture the variable within the outer
>>>            lambda (if both the inner and outer lambda have a cap default,
>>>            and the outer lambda is enclosed by a non-dependent context).
>>>          - The reason I do this in ActOnCallExpr is because until then
>>>            I do not know if the function call is still dependent, perhaps
>>>            there is a better place to do this?
>>>
>>
>> Here's how I was imagining this working when we were in CWG:
>>
>> The captures for a lambda are not finalized until the outermost context
>> of the reaching scope is non-dependent. At that point, for each call to
>> ActOnFinishFullExpr within the lambda, we check whether the full-expression
>> is instantiation-dependent. If it is, we scan it for entities which it
>> might need to capture, and capture all of them. There may be ways of
>> optimizing this (maybe keep a list of the potentially-captured variables
>> for each full-expression as we build it).
>>  *
>> *
>>
>
> Aah! ActOnFinishfullExpr was exactly the hook I was looking for - thanks!
> So the new patch implements it as you describe (and it does do the
> optimization
> - I introduced an array of potential captures in the LambdaScopeInfo) so
> we
> don't have to walk the expression tree each time.  It seems to work for
> your
> case and my previous tests.
>
> While we're on the topic, can i ask you to clarify a few capture and
> constexpr
> questions below:
>

OK, the relevant context is [basic.def.odr]p2 and p3.


> struct S {
>   constexpr S() {}
>   constexpr operator int() const { return 0; }
> };
> void fooS(S s) { }
>
> void fooInt(int i) { }
>
> void f() {
>   constexpr S s {};
>   auto L = [] (int x) {
>       (void) s;  // 1
>

Here, the 's' expression is a discarded-value expression, and the 's'
variable is in the set of potential results of the 's' expression, and 's'
satisfies the constraints for appearing in a constant expression, so 's' is
not odr-used, so is not captured. We should not require a capture-default
here.


>       fooS(s);  // 2
>

Here, 's' is implicitly passed to the implicit S::S(const S&) constructor.
This is neither an lvalue-to-rvalue conversion nor a discarded-value
expression, so 's' is odr-used and we have an error due to the missing
capture-default.


>       fooInt(s); // 3
>

This is equivalent to 's.operator int()', which again odr-uses 's', so
requires a capture-default.


>   };
> }
>
> Should the above be ok in spite of L not having a default capture?
> Currently clang errors on all of them individually - is that the right
> behavior?
>
> Also, how would you want me to start submitting patches for commit - do
> you want
> me to break up the patch into smaller patches? - and if so, do you have
> any thoughts
> on how I might want to break up the functionality per patch?
>

Smaller patches are definitely better, if you can decompose the
functionality into coherent smaller chunks. There are some hints on how to
decompose the functionality here (but this division may or may not work for
you):

http://clang.llvm.org/docs/InternalsManual.html#how-to-add-an-expression-or-statement
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130515/a0bc1075/attachment.html>