[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:63
+// FIXME: use a real Class!
+using SymbolLocation = std::variant<SourceLocation, tooling::stdlib::Symbol>;
+
----------------
hokein wrote:
> kadircet wrote:
> > let's move this to `AnalysisInternal.h`, as it isn't used in any of the public apis.
> I'm not quite sure about this --the SymbolLocation is an important concept and API, even though it is not used in public APIs. Moved anyway.
> 
FWIW +1 to the idea that this belongs in Types.h even if it's not used in public APIs at present, as discussed offline.


================
Comment at: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp:33
 TEST(WalkUsed, Basic) {
   // FIXME: Have a fixture for setting up tests.
   llvm::Annotations Code(R"cpp(
----------------
@kadircet gentle reminder - you wanted to defer this, this patch adds a copy/paste of the test setup - want me to try to pull something out before this gets out of hand?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137320