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

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added inline comments.


================
Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();
----------------
faisalv wrote:
> I think this should return an invalid range if getExplicitCount is 0.
> might assert that when not 0, langleloc does not equal rangleloc.
> 
I think it does return an invalid range in that case. Without explicit template parameters, getLAngleLoc and getRAngleLoc return invalid locations. And a range of invalid locations is an invalid range. But I suppose that could be asserted to make sure.


================
Comment at: lib/Sema/SemaLambda.cpp:495
+      reinterpret_cast<NamedDecl *const *>(TParams.begin()),
+      reinterpret_cast<NamedDecl *const *>(TParams.end()));
+  LSI->NumExplicitTemplateParams = TParams.size();
----------------
faisalv wrote:
> ack - avoid reinterpret cast please - why not just stick to Decl* for TemplateParams for now  - and add some fixme's that suggest we should consider refactoring ParseTemplateParameterList to accept a vector of nameddecls - and update this when that gets updated ?
> 
> Perhaps add an assert here that iterates through and checks to make sure each item in this list is some form of a template parameter decl - within an #ifndef NDEBUG block (or your conversion check to NameDecl should suffice?)
Unfortunately `TemplateParameterList::Create` expects an array of `NamedDecl`, so there’ll need to be a forced downcast somewhere. By getting that over with as soon as possible, any errors will be caught straight away by that assertion instead of moving the problem down the line somewhere and making it more difficult to see where it went wrong.


https://reviews.llvm.org/D36527