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

[Hamza Sood via Phabricator via cfe-commits]

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 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?
> 
Good point. It isn’t the “wrong” scope in the sense that is breaks anything, but it is a bit wasteful to unconditionally push a potentially unneeded scope.

I have another idea that could be less intrusive, which is to replace this line and the start of the if statement with:

  bool HasExplicitTemplateParams = getLangOpts().CPlusPlus2a && Tok.is(tok::less);
  ParseScope TemplateParamScope(this, Scope::TemplateParamScope, HasExplicitTemplateParams);
  if (HasExplicitTemplateParams) {
    // same code as before
  }

That way the scope is only entered when needed, but no restructuring is required (which I personally think would make things a bit harder to follow). Could that work?


https://reviews.llvm.org/D36527