hokein accepted this revision.
hokein added inline comments.
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:7
+//
+//===----------------------------------------------------------------------===//
+
----------------
would be nice to have some high-level descriptions in the file comment.
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:29
+ // FIXME: Add support for macros.
+ std::variant<Decl *, tooling::stdlib::Symbol> Storage;
+};
----------------
I see we use `Decl*` here and elsewhere (`WalkAST` etc), is there any reason of
not using `const Decl*`?
================
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:48
+/// FIXME: Provide signals about the reference type and providing headers.
+using UsedSymbolVisitor = llvm::function_ref<void(
+ SourceLocation RefLoc, Symbol Target, llvm::ArrayRef<Header> Providers)>;
----------------
nit: rename the Visitor to CB? Visitor reminds me too much of `ASTVisitor`...
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:60
-} // namespace include_cleaner
-} // namespace clang
+void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, UsedSymbolVisitor CB) {
+ tooling::stdlib::Recognizer Recognizer;
----------------
sammccall wrote:
> IMO this function really deserves its own impl file I think (at least
> Analysis.cpp) - I think it + walkAST are going to be the two biggest bundles
> of complexity in the library. (if so, same with the tests)
+1
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:71
+ // FIXME: Handle IWYU pragmas, non self-contained files.
+ llvm::errs() << "Returning fileentry\n";
+ if (auto *FE = SM.getFileEntryForID(SM.getFileID(ND.getLocation())))
----------------
nit: remove.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136293/new/
https://reviews.llvm.org/D136293
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
