[PATCH] Step 1: Simple Generic lambda (no captures or return type deduction)

[Richard Smith]

On Sat, Jun 15, 2013 at 7:34 AM, Faisal Vali <faisalv at gmail.com> wrote:

> This updated patch implements simple non-capturing, non-nested generic
> lambdas:
>
> As an example of what compiles:
>
> template <class F1, class F2>
> struct overload : F1, F2 {
>     using F1::operator();
>     using F2::operator();
>     overload(F1 f1, F2 f2) : F1(f1), F2(f2) { }
>   };
>
>   auto Recursive = [](auto Self, auto h, auto ... rest) {
>     return 1 + Self(Self, rest...);
>   };
>   auto Base = [](auto Self, auto h) {
>       return 1;
>   };
>   overload<decltype(Base), decltype(Recursive)> O(Base, Recursive);
>   int num_params =  O(O, 5, 3, "abc", 3.14, 'a');
>
> Please see attached tests for more examples.
>
>
> Changes from the initial version of this patch:
>   - added return type deduction (since the changes were minimal)
>     -- both C++11 and C++1y deduction is supported for lambdas
>     -- in c++1y mode we replace the implicit trailing return type with
> 'auto'
>         instead of 'dependent type' (c++11)
>     -- in c++1y mode forward to Richard's deduction machinery, else
>         to Doug's original deduction machinery (there are some minor
>         differences in deduction, hopefully subtly captured in the tests)
>   - Per feedback from Eli:
>     -- the ActOnAutoParameter has been buried within ActOnParamDeclarator
>     -- ConversionOperator property has been removed
>     -- "__invoke" is being used in the one place it is needed in the
> static invoker
>
> Issues:
>   - the pch test file cxx11-lambda.mm causes a crash, and I do not know
>     enough about PCH to try and tackle it without some guidance.
>  - I'm still hoping Doug or Richard or another Clang template instantiation
>    wizard can comment on the recursive instantiation issue.
>

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).


> Next TODO:
>    - once i get this patch approved and committed, i will start
> merging/working on
>      capturing capability and nested lambdas.
>
> Look forward to your timely :) review and feedback!
>
> Thanks!
>
>
> Faisal Vali
>
>
>
> On Fri, Jun 14, 2013 at 12:26 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
> > On Fri, Jun 14, 2013 at 10:01 AM, Faisal Vali <faisalv at gmail.com> wrote:
> >>
> >> On Thu, Jun 13, 2013 at 12:58 PM, Eli Friedman <eli.friedman at gmail.com>
> >> wrote:
> >> > On Tue, Jun 11, 2013 at 10:05 PM, Faisal Vali <faisalv at gmail.com>
> wrote:
> >> > @@ -547,6 +550,18 @@
> >> >
> >> >      /// \brief The type of the call method.
> >> >      TypeSourceInfo *MethodTyInfo;
> >> > +
> >> > +    /// \brief The Lambda call method
> >> > +    CXXMethodDecl *CallOperator;
> >> > +
> >> > +    /// \brief The Lambda conversion operator, for non-capturing
> >> > lambdas
> >> > +    CXXConversionDecl *ConversionOperator;
> >> > +
> >> > +    /// \brief The Lambda static method invoker, for non-capturing
> >> > lambdas
> >> > +    CXXMethodDecl *StaticInvoker;
> >> > +
> >> > +    LambdaExpr *ParentLambdaExpr;
> >> >
> >> > It's not obvious why you're making this change... we try to keep the
> AST
> >> > as
> >> > small as possible.
> >> >
> >>
> >> I see, but how do you suggest I access this data when I need it?
> >
> >
> > If you really do need it all, it's okay, I guess... it was just a bit
> > surprising at first glance.  Taking another look, it doesn't look like
> > you're actually using getLambdaConversionOperator() (and in clang's
> > implementation, lambdas can have multiple conversion operators).
> >
> >>
> >>
> >> >
> >> > +      ParmVarDecl *PVD = cast<ParmVarDecl>(Param);
> >> > +      // Handle 'auto' within a generic lambda.
> >> > +      // FVQUESTION: Should I use getOriginalType here - how do i
> >> > +      // know when to use which?
> >> > +      QualType ParamType = PVD->getType();
> >> >
> >> > If you're unsure, getType() is probably the right choice... I don't
> >> > think it
> >> > matters here, though.
> >> >
> >>
> >> Thanks!  What is the difference between the two?
> >
> >
> > getOriginalType() returns the undecayed type; getType() returns the
> semantic
> > type of the declaration.
> >
> >>
> >>
> >> > +      if (getLangOpts().CPlusPlus1y &&
> >> > ParamType->getContainedAutoType()) {
> >> > +        TemplateTypeParmDecl *TemplateParam =
> >> > +            Actions.ActOnLambdaAutoParameter(getCurScope(), PVD,
> >> > +                          CurTemplateDepth, CurAutoParameterIndex);
> >> > +        TemplateParams.push_back(TemplateParam);
> >> > +      }
> >> >
> >> > We generally prefer to avoid having code in Parser/ inspect AST types.
> >> > Can
> >> > you move building the TemplateParameterList into
> >> > ActOnStartOfLambdaDefinition?
> >> >
> >>
> >>
> >> In my initial implementation of generic lambdas, I had encased this
> >> transformation
> >> entirely within ActOnStartOfLambdaDefinition (which is called once the
> >> parameter
> >> declaration clause has been constructed).   In that version, I
> >> reconstructed
> >> the typesourceinfo and function type of the call operator after it was
> >> initially
> >> parsed and semanalyzed, by replacing each auto with a template type
> >> paramater,
> >> creating the template prameter list, and using a visitor to fix all
> >> subsequent references (ugh!)
> >>
> >> Based on sage advice from Doug, I moved the transformation upstream
> >> and the code changes
> >> have proved to be much leaner.
> >
> >
> > Okay.
> >
> >> In regards to avoiding having the Parser inspect AST types What I
> >> could do is nest
> >> ActOnAutoParameter within ActOnParamDeclarator.  Is it ok that I pass
> >> in a vector
> >> of TemplateTypeParamDecl* as an out parameter to
> >> ParseParameterDeclarationClause
> >> which I will then need to pass in to ActOnParamDeclarator? Or is it
> >> important that I find a way to do it without passing that vector around?
> >
> >
> > That should be fine.
> >
> >>
> >> > +// The name of the static invoker function that will forward
> >> > +// to the corresponding lambda call operator.
> >> > +static inline const char* getSecretLambdaStaticInvokerStringID() {
> >> > +  return "__invoker";
> >> > +}
> >> >
> >> > I'm not sure how this abstraction is actually helpful, given the
> >> > function
> >> > only has one use.
> >> >
> >>
> >> I figured it might be less brittle if more uses cropped up in the
> future.
> >> But
> >> I can just insert the name: "__invoker" in the one place i need it, and
> >> drop
> >> this function?
> >
> >
> > I think so.  We can always change it when we add more uses.
> >
> >>
> >> > +  // FVQUESTION? When a generic lamdba call operator is being
> >> > instantiated,
> >> > if
> >> > +  // for some reason instantiation fails, the SemaDiagnosticEmitter
> >> > ends up
> >> > +  // emitting the entire instantiation stack, and since local pending
> >> > +  // instantiations are called recursively,
> >> >
> >> > We should not be instantiating the lambda in your example more than
> once
> >> > at
> >> > the same time.  If we are, it's probably a bug.
> >> >
> >>
> >>
> >> Does my workaround seem reasonable?
> >>
> >
> > Sorry, I'm not really an expert on template instantiation.  Hopefully
> Doug
> > will chime in.
> >
> > -Eli
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130615/ceec658a/attachment.html>