[PATCH] Step 1: Simple Generic lambda (no captures or nested lambdas)

Faisal Vali faisalv at gmail.com
Sun Jun 16 07:24:13 PDT 2013


This updated patch implements simple non-capturing, non-nested generic
lambdas, with return type deduction.  The changes from the previous patch:

- added basic serialization/deserialization code (all pch tests pass, i added
some simple generic lambda ones)
   - added support for serializing/deserializing the CXXRecordDecl
     LambdaDataDefinition members I added
   - also added ser/deser support for PendingLocalImplicitInstantiations
     (else the uninstantiated generic lambdas create inconsistencies with
      pch)

- addressed the recursive instantiation bug (which only seemed to manifest
during diagnostic emission) with the much cleaner fix suggested by richard
(swapping the localpendinginstantiations in and out)


All clang tests pass (except for the ones having to do with Index and Modules
that failed for me prior to the changes)

Can someone ;) please give me a sense of how far this is from being
commit-ready?

Thanks!
Faisal Vali



On Sat, Jun 15, 2013 at 3:04 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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 --------------
A non-text attachment was scrubbed...
Name: simple-GL+return-deduction+fix-pch&recursive-template.patch
Type: application/octet-stream
Size: 82186 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130616/4da0ebd6/attachment.obj>


More information about the cfe-commits mailing list