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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
