[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    [PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas
From:       Richard Smith - zygoloid via Phabricator via cfe-commits <cfe-commits () lists ! llvm
Date:       2017-09-30 0:43:19
Message-ID: e2353f25500c0cbb4f9295a9e2e2ea37 () localhost ! localdomain
[Download RAW message or body]

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.


https://reviews.llvm.org/D36527



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]