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

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 29 17:43:19 PDT 2017

rsmith added a comment.

A couple of remaining pieces that I think are missing:

- Tests for instantiation of templates containing one of these lambdas. I would expect you'll find you need to change Sema/TreeTransform.h to make that work.
- Updates to Itanium mangling (AST/ItaniumMangle.cpp) for the mangling changes described in https://github.com/itanium-cxx-abi/cxx-abi/issues/31

Comment at: lib/Parse/ParseExprCXX.cpp:1117
+                                /*EnteredScope=*/HasExplicitTemplateParams);
+  if (HasExplicitTemplateParams) {
+    SmallVector<NamedDecl*, 4> TemplateParams;
Please add a pre-C++2a-compat warning along this code path, and permit this in pre-C++2a modes with an extension warning. (You can look at other C++2a features for examples of how to do this.)

Comment at: lib/Parse/ParseExprCXX.cpp:1151-1165
+      // Record the template parameter depth for auto parameters.
+      // Subtract 1 from the current depth if we've parsed an explicit template
+      // parameter list, because that will have increased the depth.
+      Actions.RecordParsingTemplateParameterDepth(HasExplicitTemplateParams
+                                                    ? TemplateParameterDepth - 1
+                                                    : TemplateParameterDepth);
This handling of the template depth is more subtle / error-prone than I'd like. Maybe you could add a `TemplateParameterDepthRAII::setAddedDepth(1)` to replace the conditional increment, and a `TemplateParameterDepthRAII::getOriginalDepth()` to replace the conditional subtraction of 1?

Comment at: lib/Sema/SemaLambda.cpp:484-490
+  assert(LSI->NumExplicitTemplateParams == 0
+         && "Already acted on explicit template parameters");
+  assert(LSI->TemplateParams.size() == 0
+         && "Explicit template parameters should come "
+            "before invented (auto) ones");
+  assert(TParams.size() > 0
+         && "No template parameters to act on");
Please put the `&&` on the previous line.


More information about the cfe-commits mailing list