[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rL359967: [c++20] Implement P0428R2 - Familiar template syntax for generic lambdas (authored by hamzasood, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith accepted this revision. rsmith marked an inline comment as done. rsmith added a comment. This revision is now accepted and ready to land. Thank you, please go ahead and land this! (We'll need to parse and handle //requires-clause//s here too, but that can wait for another patch.) (Please

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood marked 6 inline comments as done. hamzasood added inline comments. Comment at: lib/AST/DeclPrinter.cpp:107-108 -void printTemplateParameters(const TemplateParameterList *Params); +void printTemplateParameters(const TemplateParameterList *Params, +

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 177221. hamzasood added a comment. - Committed the test framework changes separately as r348589 and r348603. - Correctly cleanup the lambda scope if an error in encountered while parsing the template parameters. - Replaced

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

erichkeane added a comment. Submitter: Any update on the status? It seems that Richard's feedback should be trivial enough, so I was wondering if you need this taken up? https://reviews.llvm.org/D36527 ___ cfe-commits mailing list

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith added a comment. Herald added a subscriber: arphaman. This looks great, thanks! In https://reviews.llvm.org/D36527#892395, @hamzasood wrote: > Could you expand on your first point a bit more? Do you have an example that > shows the issue? You have very little test coverage for the

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 156693. hamzasood added a comment. - Resolved merge conflicts https://reviews.llvm.org/D36527 Files: include/clang/AST/DeclCXX.h include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 123015. hamzasood added a comment. Resolved merge conflicts from the last month of changes (`unittests/AST/StmtPrinterTest.cpp` and `lib/AST/DeclCXX.cpp`) https://reviews.llvm.org/D36527 Files: include/clang/AST/DeclCXX.h

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 118261. hamzasood added a comment. - Updated lambda mangling to include explicit template parameters - Allow explicit template parameter lists on lambdas pre-c++2a as an extension. - Improved the somewhat fragile template depth handling. - Reformatted some

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith added a comment. A couple of remaining pieces that I think are missing: - Tests for instantiation of templates containing one of these lambdas. I would expect you'll find you need to change Sema/TreeTransform.h to make that work. - Updates to Itanium mangling (AST/ItaniumMangle.cpp) for

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 114550. hamzasood added a comment. - Replaced an unneeded `STLExtras.h` include with ``. - Assert that the lambda template scope has a parent before accessing it. - Added an equals in a braced init as per the LLVM coding standards. Thanks @Rakete

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

Rakete added a comment. > Is that a problem? I don't think so, because `shouldVisitImplicitCode()` doesn't mean that it only visits implicit code, it's not `onlyVisitImplicitCode()` :) In fact, I would be surprised if the template parameters were only visited once.

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Ping https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 113271. hamzasood added a comment. Herald added a subscriber: klimek. Implemented pretty printing and recursive AST visiting (both with appropriate unit tests). The pretty printing implementation includes fixing a few printing bugs (which I needed fixed

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. In https://reviews.llvm.org/D36527#854883, @faisalv wrote: > > Interestingly we don't (explicitly) visit the lambda's function parameters? This is obviously false - sorry :) https://reviews.llvm.org/D36527 ___

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. In https://reviews.llvm.org/D36527#854600, @rsmith wrote: > This patch appears to be missing some necessary changes in the following > places: > > - lib/Serialization -- round-trip the new information through PCH / modules Shouldn't the explicit template parameter

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith added a comment. In https://reviews.llvm.org/D36527#854600, @rsmith wrote: > We'll also need to agree within the cxx-abi-dev list how to mangle lambda > closure types for this form I filed https://github.com/itanium-cxx-abi/cxx-abi/issues/31 for this. https://reviews.llvm.org/D36527

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith added a comment. This patch appears to be missing some necessary changes in the following places: - lib/Serialization -- round-trip the new information through PCH / modules - lib/AST/StmtPrinter.cpp -- pretty-printing lambdas with explicit template parameters - lib/Sema/TreeTransform.h

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 112867. hamzasood added a comment. Hi Richard, thanks for your feedback. This update makes the changes that you requested. It turns out that the casting is no longer an issue since `ParseTemplateParameters` has been refactored to use `NamedDecl`.

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rsmith added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:638 /// lambda-introducer lambda-declarator[opt] compound-statement +/// lambda-introducer lambda-declarator[opt] +/// compound-statement We generally put

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Thanks for all of your feedback! It’s really helped to improve this patch. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList();

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList(); hamzasood wrote: > faisalv wrote: > > I think this should

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. OK - I'll commit this on sunday if no one blocks it by then. (I'll add the fixme's). Nice Work! https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList(); faisalv wrote: > I think this should return an invalid

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList(); I think this should return an invalid range if

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 112018. hamzasood added a comment. - Info about a lambda's explicit template parameters is now exposed in the AST (see changes to LambdaExpr). It turns out that no extra storage was actually needed to achieve this. - Removed remnants of a previous

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1116 + if (HasExplicitTemplateParams) { +SmallVector TemplateParams; +SourceLocation LAngleLoc, RAngleLoc; hamzasood wrote: > faisalv wrote: > > hamzasood wrote: > > >

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/Sema/SemaLambda.cpp:858 +KnownDependent = CurScope->getTemplateParamParent() != nullptr; + } hamzasood wrote: > faisalv wrote: > > Hmm - now that you drew my attention to this ;) - I'm pretty sure this is >

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1116 + if (HasExplicitTemplateParams) { +SmallVector TemplateParams; +SourceLocation LAngleLoc, RAngleLoc; faisalv wrote: > hamzasood wrote: > > faisalv wrote: > > > Why

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1116 + if (HasExplicitTemplateParams) { +SmallVector TemplateParams; +SourceLocation LAngleLoc, RAngleLoc; hamzasood wrote: > faisalv wrote: > > Why not Just use/pass

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood marked 5 inline comments as done. hamzasood added a comment. So to clarify: NumExplicitTemplateParams should be stored in LambdaDefinitionData (with an accessor in LambdaExpr) and ExplicitTemplateParamRange (SourceRange) should be stored in LambdaExpr? That makes sense, but doesn't

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. In regards to representing this in the AST - I think (based on precedence) that the number of explicit template parameters should be stored in LambdaDefinitionData - and the interface exposed through LambdaExpr (where the source information of the template parameter

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 111892. hamzasood added a comment. Sorry, I've just spotted a small mistake in how the auto parameter depth is recorded. This update fixes it. https://reviews.llvm.org/D36527 Files: include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 111887. hamzasood added a comment. - Corrected a typo. - Made HasExplicitTemplateParams const. - Reverted the change in where template depth is incremented and recorded. - Fixed the broken test. - Keep the template scope active instead of exiting it and

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1090 + TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth); + Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth); + hamzasood wrote: > faisalv wrote:

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1090 + TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth); + Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth); + faisalv wrote: > Since you only

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. In regards to that failing test (that was added since review began) - could you fix that test pls (i.e. rename the nested ttp 'U' to something else) and move it into the function 'f' as requested by the author? Might want to include a similar (but not same) example of

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: include/clang/Sema/ScopeInfo.h:774 + /// \brief The number of parameters in the template parameter list that were + /// explicitely specified by the user, as opposed to being invented by use + /// of an auto parameter.

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1112 + ParseScope TemplateParamScope(this, Scope::TemplateParamScope); + if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) { hamzasood wrote: > faisalv wrote: > > We always create a

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1112 + ParseScope TemplateParamScope(this, Scope::TemplateParamScope); + if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) { faisalv wrote: > We always create a template parameter

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added inline comments. Comment at: include/clang/Sema/Sema.h:5466 + /// ActOnLambdaTemplateParameterList - This is called after parsing + /// the explicit template parameter list (if it exists) in C++2a. Avoid listing the name of the function:

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

faisalv added a comment. I'll try and get you some feedback on this over the next couple of days (unless someone else jumps in). Thanks for working on this! https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

rjmccall resigned from this revision. rjmccall added a comment. I don't think I'm best-equipped to review this, sorry. https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Ping https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood created this revision. This patch provides an implementation for P0428R2 . https://reviews.llvm.org/D36527 Files: include/clang/Basic/DiagnosticParseKinds.td include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp