[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:1116
+  if (HasExplicitTemplateParams) {
+    SmallVector<Decl*, 4> TemplateParams;
+    SourceLocation LAngleLoc, RAngleLoc;
----------------
faisalv wrote:
> hamzasood wrote:
> > faisalv wrote:
> > > Why not Just use/pass LSI->TemplateParams?
> > I thought that Parser and Sema stay separate, and communicate through various ActOn functions? Directly accessing LSI would violate that.
> Aah yes - you're right.  Still it does seem a little wasteful to create two of those (and then append).  What are your thoughts about passing the argument by (moved from) value, and then swapping their guts within ActOn (i..e value-semantics) ?  (I suppose the only concern would be the small array case - but i think that would be quite easy for any optimizer to inline).   
> 
I don't think a SmallVectorImpl can be passed by value.

So to make that work, the function would either needed to be templated (SmallVector<Decl*, I>) or only accept a SmallVector<Decl*, 4>. And I don't think either of those options are worthwhile.


================
Comment at: lib/Sema/SemaLambda.cpp:858
+    KnownDependent = CurScope->getTemplateParamParent() != nullptr;
+  }
 
----------------
faisalv wrote:
> Hmm - now that you drew my attention to this ;) - I'm pretty sure this is broken - but (embarrassingly) it broke back when i implemented generic lambdas (and interestingly is less broken w generic lambdas w explicit TPLs) - could I ask you to add a FIXME here that states something along the lines of:  
> 
> When parsing default arguments that contain lambdas, it seems necessary to know whether the containing parameter-declaration clause is that of a template to mark the closure type created in the default-argument as dependent.  Using template params to detect dependence is not enough for all generic lambdas since you can have generic lambdas without explicit template parameters, and whose default arguments contain lambdas that should be dependent - and you can not rely on the existence of a template parameter scope to detect those cases.  Consider:
>    auto L = [](int I = [] { return 5; }(), auto a) { };  
> The above nested closure type (of the default argument) occurs within a dependent context and is therefore dependent - but we won't know that until we parse the second parameter.  
> 
> p.s. I'll try and get around to fixing this if no one else does.
> 
Good point. Now you mention it, isn't it even more broken than than?
E.g:

```
 auto L = [](auto A, int I = [] { return 5; }());
```

L is known to be generic before we parse the nested lambda, but template param scopes aren't established for auto parameters (I think), so the nested lambda won't get marked as dependent


https://reviews.llvm.org/D36527