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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Parse/ParseExprCXX.cpp:638
 ///         lambda-introducer lambda-declarator[opt] compound-statement
+///         lambda-introducer <template-parameter-list> lambda-declarator[opt]
+///             compound-statement
----------------
We generally put terminals in single quotes in these grammar excerpts:
```
  lambda-introducer '<' template-parameter-list '>' lambda-declarator[opt]
```


================
Comment at: lib/Parse/ParseExprCXX.cpp:1126-1127
+           diag::err_lambda_template_parameter_list_empty);
+    }
+    else {
+      Actions.ActOnLambdaExplicitTemplateParameterList(
----------------
No linebreak between `}` and `else`.


================
Comment at: lib/Sema/SemaLambda.cpp:495
+      reinterpret_cast<NamedDecl *const *>(TParams.begin()),
+      reinterpret_cast<NamedDecl *const *>(TParams.end()));
+  LSI->NumExplicitTemplateParams = TParams.size();
----------------
hamzasood wrote:
> faisalv wrote:
> > ack - avoid reinterpret cast please - why not just stick to Decl* for TemplateParams for now  - and add some fixme's that suggest we should consider refactoring ParseTemplateParameterList to accept a vector of nameddecls - and update this when that gets updated ?
> > 
> > Perhaps add an assert here that iterates through and checks to make sure each item in this list is some form of a template parameter decl - within an #ifndef NDEBUG block (or your conversion check to NameDecl should suffice?)
> Unfortunately `TemplateParameterList::Create` expects an array of `NamedDecl`, so there’ll need to be a forced downcast somewhere. By getting that over with as soon as possible, any errors will be caught straight away by that assertion instead of moving the problem down the line somewhere and making it more difficult to see where it went wrong.
OK, but please cast them one by one rather than type-punning the array (which results in UB).


================
Comment at: lib/Sema/SemaLambda.cpp:845-846
+                                       ->getTemplateParamParent() != nullptr;
+  }
+  else {
+    KnownDependent = CurScope->getTemplateParamParent() != nullptr;
----------------
No linebreak between `}` and `else`.


================
Comment at: test/CXX/temp/temp.decls/temp.variadic/p4.cpp:216
 
+#if __cplusplus >= 201707L
       //    - in a template parameter pack that is a pack expansion
----------------
Please use `__cplusplus > 201703L` rather than a number that will become meaningless once we have a real value for C++2a.


https://reviews.llvm.org/D36527