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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
