On Sat, Jun 15, 2013 at 7:34 AM, Faisal Vali <span dir="ltr"><<a href="mailto:faisalv@gmail.com" target="_blank">faisalv@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This updated patch implements simple non-capturing, non-nested generic lambdas:<br>
<br>
As an example of what compiles:<br>
<br>
template <class F1, class F2><br>
struct overload : F1, F2 {<br>
using F1::operator();<br>
using F2::operator();<br>
overload(F1 f1, F2 f2) : F1(f1), F2(f2) { }<br>
};<br>
<br>
auto Recursive = [](auto Self, auto h, auto ... rest) {<br>
return 1 + Self(Self, rest...);<br>
};<br>
auto Base = [](auto Self, auto h) {<br>
return 1;<br>
};<br>
overload<decltype(Base), decltype(Recursive)> O(Base, Recursive);<br>
int num_params = O(O, 5, 3, "abc", 3.14, 'a');<br>
<br>
Please see attached tests for more examples.<br>
<br>
<br>
Changes from the initial version of this patch:<br>
- added return type deduction (since the changes were minimal)<br>
-- both C++11 and C++1y deduction is supported for lambdas<br>
-- in c++1y mode we replace the implicit trailing return type with 'auto'<br>
instead of 'dependent type' (c++11)<br>
-- in c++1y mode forward to Richard's deduction machinery, else<br>
to Doug's original deduction machinery (there are some minor<br>
differences in deduction, hopefully subtly captured in the tests)<br>
- Per feedback from Eli:<br>
-- the ActOnAutoParameter has been buried within ActOnParamDeclarator<br>
-- ConversionOperator property has been removed<br>
-- "__invoke" is being used in the one place it is needed in the<br>
static invoker<br>
<br>
Issues:<br>
- the pch test file <a href="http://cxx11-lambda.mm" target="_blank">cxx11-lambda.mm</a> causes a crash, and I do not know<br>
enough about PCH to try and tackle it without some guidance.<br>
- I'm still hoping Doug or Richard or another Clang template instantiation<br>
wizard can comment on the recursive instantiation issue.<br></blockquote><div><br></div><div>I think you just need to save and clear PendingLocalImplicitInstantiations before you start instantiating a function definition, and restore it afterwards (just like we already do for PendingInstantiations, but not conditional on the 'Recursive' flag).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Next TODO:<br>
- once i get this patch approved and committed, i will start<br>
merging/working on<br>
capturing capability and nested lambdas.<br>
<br>
Look forward to your timely :) review and feedback!<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
Faisal Vali<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On Fri, Jun 14, 2013 at 12:26 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>
> On Fri, Jun 14, 2013 at 10:01 AM, Faisal Vali <<a href="mailto:faisalv@gmail.com">faisalv@gmail.com</a>> wrote:<br>
>><br>
>> On Thu, Jun 13, 2013 at 12:58 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>><br>
>> wrote:<br>
>> > On Tue, Jun 11, 2013 at 10:05 PM, Faisal Vali <<a href="mailto:faisalv@gmail.com">faisalv@gmail.com</a>> wrote:<br>
>> > @@ -547,6 +550,18 @@<br>
>> ><br>
>> > /// \brief The type of the call method.<br>
>> > TypeSourceInfo *MethodTyInfo;<br>
>> > +<br>
>> > + /// \brief The Lambda call method<br>
>> > + CXXMethodDecl *CallOperator;<br>
>> > +<br>
>> > + /// \brief The Lambda conversion operator, for non-capturing<br>
>> > lambdas<br>
>> > + CXXConversionDecl *ConversionOperator;<br>
>> > +<br>
>> > + /// \brief The Lambda static method invoker, for non-capturing<br>
>> > lambdas<br>
>> > + CXXMethodDecl *StaticInvoker;<br>
>> > +<br>
>> > + LambdaExpr *ParentLambdaExpr;<br>
>> ><br>
>> > It's not obvious why you're making this change... we try to keep the AST<br>
>> > as<br>
>> > small as possible.<br>
>> ><br>
>><br>
>> I see, but how do you suggest I access this data when I need it?<br>
><br>
><br>
> If you really do need it all, it's okay, I guess... it was just a bit<br>
> surprising at first glance. Taking another look, it doesn't look like<br>
> you're actually using getLambdaConversionOperator() (and in clang's<br>
> implementation, lambdas can have multiple conversion operators).<br>
><br>
>><br>
>><br>
>> ><br>
>> > + ParmVarDecl *PVD = cast<ParmVarDecl>(Param);<br>
>> > + // Handle 'auto' within a generic lambda.<br>
>> > + // FVQUESTION: Should I use getOriginalType here - how do i<br>
>> > + // know when to use which?<br>
>> > + QualType ParamType = PVD->getType();<br>
>> ><br>
>> > If you're unsure, getType() is probably the right choice... I don't<br>
>> > think it<br>
>> > matters here, though.<br>
>> ><br>
>><br>
>> Thanks! What is the difference between the two?<br>
><br>
><br>
> getOriginalType() returns the undecayed type; getType() returns the semantic<br>
> type of the declaration.<br>
><br>
>><br>
>><br>
>> > + if (getLangOpts().CPlusPlus1y &&<br>
>> > ParamType->getContainedAutoType()) {<br>
>> > + TemplateTypeParmDecl *TemplateParam =<br>
>> > + Actions.ActOnLambdaAutoParameter(getCurScope(), PVD,<br>
>> > + CurTemplateDepth, CurAutoParameterIndex);<br>
>> > + TemplateParams.push_back(TemplateParam);<br>
>> > + }<br>
>> ><br>
>> > We generally prefer to avoid having code in Parser/ inspect AST types.<br>
>> > Can<br>
>> > you move building the TemplateParameterList into<br>
>> > ActOnStartOfLambdaDefinition?<br>
>> ><br>
>><br>
>><br>
>> In my initial implementation of generic lambdas, I had encased this<br>
>> transformation<br>
>> entirely within ActOnStartOfLambdaDefinition (which is called once the<br>
>> parameter<br>
>> declaration clause has been constructed). In that version, I<br>
>> reconstructed<br>
>> the typesourceinfo and function type of the call operator after it was<br>
>> initially<br>
>> parsed and semanalyzed, by replacing each auto with a template type<br>
>> paramater,<br>
>> creating the template prameter list, and using a visitor to fix all<br>
>> subsequent references (ugh!)<br>
>><br>
>> Based on sage advice from Doug, I moved the transformation upstream<br>
>> and the code changes<br>
>> have proved to be much leaner.<br>
><br>
><br>
> Okay.<br>
><br>
>> In regards to avoiding having the Parser inspect AST types What I<br>
>> could do is nest<br>
>> ActOnAutoParameter within ActOnParamDeclarator. Is it ok that I pass<br>
>> in a vector<br>
>> of TemplateTypeParamDecl* as an out parameter to<br>
>> ParseParameterDeclarationClause<br>
>> which I will then need to pass in to ActOnParamDeclarator? Or is it<br>
>> important that I find a way to do it without passing that vector around?<br>
><br>
><br>
> That should be fine.<br>
><br>
>><br>
>> > +// The name of the static invoker function that will forward<br>
>> > +// to the corresponding lambda call operator.<br>
>> > +static inline const char* getSecretLambdaStaticInvokerStringID() {<br>
>> > + return "__invoker";<br>
>> > +}<br>
>> ><br>
>> > I'm not sure how this abstraction is actually helpful, given the<br>
>> > function<br>
>> > only has one use.<br>
>> ><br>
>><br>
>> I figured it might be less brittle if more uses cropped up in the future.<br>
>> But<br>
>> I can just insert the name: "__invoker" in the one place i need it, and<br>
>> drop<br>
>> this function?<br>
><br>
><br>
> I think so. We can always change it when we add more uses.<br>
><br>
>><br>
>> > + // FVQUESTION? When a generic lamdba call operator is being<br>
>> > instantiated,<br>
>> > if<br>
>> > + // for some reason instantiation fails, the SemaDiagnosticEmitter<br>
>> > ends up<br>
>> > + // emitting the entire instantiation stack, and since local pending<br>
>> > + // instantiations are called recursively,<br>
>> ><br>
>> > We should not be instantiating the lambda in your example more than once<br>
>> > at<br>
>> > the same time. If we are, it's probably a bug.<br>
>> ><br>
>><br>
>><br>
>> Does my workaround seem reasonable?<br>
>><br>
><br>
> Sorry, I'm not really an expert on template instantiation. Hopefully Doug<br>
> will chime in.<br>
><br>
> -Eli<br>
><br>
</div></div></blockquote></div><br>