[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:41
 
-  void report(SourceLocation Loc, NamedDecl *ND) {
+  void report(SourceLocation Loc, NamedDecl *ND, RefType RT) {
     if (!ND || Loc.isInvalid())
----------------
using default arguments here would reduce the noise level a lot


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:69
+    llvm::for_each(E->decls(), [this, E](NamedDecl *D) {
+      report(E->getNameLoc(), D, RefType::Related);
+    });
----------------
This doesn't correspond to the example given for RefKind: the docs suggest they are *unused* while here we don't know if they're used.

At least for this case it seems calling the RefKind `Ambiguous` would also be clearer.


================
Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:107
 TEST(WalkAST, DeclRef) {
-  testWalk("int ^x;", "int y = ^x;");
-  testWalk("int ^foo();", "int y = ^foo();");
-  testWalk("namespace ns { int ^x; }", "int y = ns::^x;");
-  testWalk("struct S { static int ^x; };", "int y = S::^x;");
+  testWalk("int $explicit^x;", "int y = ^x;");
+  testWalk("int $explicit^foo();", "int y = ^foo();");
----------------
This is very noisy. please find a way to avoid having to mark all test cases as `$explicit`, e.g. by splitting implicit cases into different tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135859