[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:29
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include <cstdint>
+#include <utility>
----------------
unused?

(flagging these in case these are tool-assisted but the tool is buggy)


================
Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:30
+#include <cstdint>
+#include <utility>
+#include <variant>
----------------
unused?


================
Comment at: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp:52
+          Inputs.ExtraArgs.push_back("-std=c++17");
+          return TestAST(Inputs);
+        }()) {}
----------------
or maybe clearer `return Inputs`, and you're calling AST(Inputs) as the member initializer


================
Comment at: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp:75
+      ADD_FAILURE() << "Couldn't find any decls with name: " << SymbolName;
+    assert(DeclToLocate);
+    return *DeclToLocate;
----------------
nit: you have an assertion here but not in findMacro


================
Comment at: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp:112
+    LocateExample Test(Case);
+    EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+                ElementsAreArray(Test.points()));
----------------
<< Case, or SCOPED_TRACE, or something so we can see what test it is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135953