[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:1090
+  TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
+  Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth);
+
----------------
faisalv wrote:
> Since you only really need to pass this information on for computing the depth of the 'auto' parameters - why not just leave the RecordParsingTEmplateParameterDepth call where it was, increment the template depth once we parse the TPL, and just pass in the right depth (minus one if ExplicitTemplateParameters) and increment the tracker if we getGenericLambda but no explicit TPL?
> 
> I wonder if that might be the safer way to do it - especially if you have generic lambdas in default arguments of generic lambdas - each of which have explicit template parameters also??
> 
> thoughts?
> 
While the depth is currently only used for auto parameters, that could change in the future (especially since the function name is quite general. I.e. record depth instead of record auto depth or whatever). Because of that, it wouldn't seem right to not record it in the case where there's an explicit template param list but no function parameters. So the options were to record it twice (parsing the explicit TPL and function parameters) or just do it once for every lambda. Since it's a cheap operation (pretty much just setting a variable), I went for the latter.

Good point about incrementing after seeing an explicit template param list. I'll change that in the next update.


================
Comment at: lib/Parse/ParseExprCXX.cpp:1305
 
+  TemplateParamScope.Exit();
+
----------------
faisalv wrote:
> Why do you exit the scope here, and then re-add the template parameters to the current scope?  What confusion (if any) occurs if you leave this scope on?
> 
That was my original attempt, but I just couldn't get it to work.

E.g.

```
auto l = []<typename T>(T t) { };
```

triggers an assertion:

```
(!A->getDeducedType().isNull() && "cannot request the size of an undeduced or dependent auto type"), function getTypeInfoImpl, file /Users/hamzasood/LLVM/src/tools/clang/lib/AST/ASTContext.cpp, line 1883.
```

A lot of other things broke too.

I figured since function parameters are explicitly added to scope, it would make sense to introduce explicit template parameters in the same place.


https://reviews.llvm.org/D36527