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

[Blitz Rakete via Phabricator via cfe-commits]

Rakete1111 added a comment.

> Is that a problem?

I don't think so, because `shouldVisitImplicitCode()` doesn't mean that it only visits implicit code, it's not `onlyVisitImplicitCode()` :) In fact, I would be surprised if the template parameters were only visited once.



================
Comment at: lib/AST/ExprCXX.cpp:21
 #include "clang/Basic/IdentifierTable.h"
+#include "llvm/ADT/STLExtras.h"
 using namespace clang;
----------------
You don't need this header.


================
Comment at: lib/AST/ExprCXX.cpp:1010
+  return std::count_if(List->begin(), List->end(),
+                       [](const NamedDecl *D) { return !D->isImplicit(); });
 }
----------------
You could store the lambda in a variable instead of having two times the same exact lambda expression.


================
Comment at: lib/Sema/SemaLambda.cpp:840
+              "template param scope");
+    KnownDependent = TemplateParamScope->getParent()
+                                       ->getTemplateParamParent() != nullptr;
----------------
I think you should add an `assert` to verify that `getParent()` doesn't return `nullptr`, just to be safe from UB.


================
Comment at: unittests/AST/StmtPrinterTest.cpp:126
+                     const T &NodeMatch, StringRef ExpectedPrinted) {
+  std::vector<std::string> Args {
+    "-std=c++98",
----------------
LLVM style guide says that if you are using a braced-init-list to initialize an object, you have to use an `=`.


https://reviews.llvm.org/D36527