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

[Faisal Vali]

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:
>>
>> Hello,
>>        Attached is a patch that implements a very simple form of generic
>> lambdas
>> that allows 'auto' and variadic 'auto' as parameters in lambdas.  This
>> allows the
>> following code to compile:
>>   auto Fact = [](auto Self, unsigned n) -> unsigned {
>>     return !n ? 1 : Self(Self, n - 1) * n;
>> };
>> auto six = Fact(Fact, 3);
>
>
> Index: lib/Serialization/ASTReader.cpp
> ===================================================================
> --- lib/Serialization/ASTReader.cpp (revision 183717)
> +++ lib/Serialization/ASTReader.cpp (working copy)
> @@ -4704,7 +4704,10 @@
>      QualType Deduced = readType(*Loc.F, Record, Idx);
>      bool IsDecltypeAuto = Record[Idx++];
>      bool IsDependent = Deduced.isNull() ? Record[Idx++] : false;
> -    return Context.getAutoType(Deduced, IsDecltypeAuto, IsDependent);
> +    //FIXME: How do we figure out if this needs to be a parameter pack?
>
> By making the corresponding change to ASTWriter.cpp.
>


Thanks. Will do.


> @@ -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?

>
> +      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?

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

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?


> +// 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?


> +  // 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?

Thank you Eli for taking the time to review this!