[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LGTM from my side, a few optional NITs.
Feel free to land as soon as @hokein stamps.



================
Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:16
+
+// Check to ensure that CXXCtorInitializer is not visited when implicit code
+// should not be visited and that it is visited when implicit code should be
----------------
NIT: this comment is about the `TEST` rather than the helper class. Maybe move it there?


================
Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:29
+    if (!Init->isWritten())
+      VisitedImplicitInitializer = true;
+    Match("initializer", Init->getSourceLocation());
----------------
NIT: alternatively use different match identifiers for written and unwritten initializers:
```
if (Init->isWritten())
  Match("written-inititiazlier", ...);
else
  Match("implicit-initializer", ...);

...
TEST() {
  Visitor.expectMatch("written-initializer");
  Visitor.disallowMatch("implicit-initializer"); // would that work with invalid source locs, though?
}
```

This allows to reuse the mechanism used by other tests without extra code. But up to you, definitely not a big deal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65735/new/

https://reviews.llvm.org/D65735