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

[Faisal Vali via Phabricator via cfe-commits]

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: replace a w a \brief comment.


================
Comment at: lib/Parse/ParseExprCXX.cpp:1112
 
+  ParseScope TemplateParamScope(this, Scope::TemplateParamScope);
+  if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) {
----------------
We always create a template parameter scope here - but it's only the right scope if we have explicit template parameters, correct?

What I think we should do here is :
  - factor out (preferably as a separate patch) the post explicit-template-parameter-list parsing into a separate function (F1) which is then called from here.
  - then in this patch factor out the explicit template-parameter-list parsing also into a separate function that then either calls the function above ('F1'), or sequenced these statements before a call to 'F1'
  - also since gcc has had explicit template parameters on their generic lambdas for a while, can we find out under what options they have it enabled, and consider enabling it under those options for our gcc emulation mode? (or add a fixme for it?)
  - should we enable these explicit template parameters for pre-C++2a modes and emit extension/compatibility warnings where appropriate?



================
Comment at: lib/Parse/ParseExprCXX.cpp:1123
+      Diag(RAngleLoc,
+           diag::err_expected_lambda_template_parameter_list);
+    }
----------------
I think it might be more user friendly if you used a different error message for the <> case here - along the lines of: empty template parameter list is not allowed 


https://reviews.llvm.org/D36527