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

[Eli Friedman]

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/20130614/29eedb03/attachment.html>