This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cf888514454: [include-cleaner] Add self-contained file
support for PragmaIncludes. (authored by hokein).
Repository:
rG LLVM Github Monorepo
hokein added inline comments.
Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47
+ }
+ void buildAST() { AST = std::make_unique(Inputs); }
+
kadircet wrote:
> nit: i'd actually rename this to `astWithIncludes` and take in a set of
hokein updated this revision to Diff 476425.
hokein marked 3 inline comments as done.
hokein added a comment.
update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
https://reviews.llvm.org/D137698
Files:
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks, lgtm!
Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47
+ }
+ void buildAST() { AST = std::make_unique(Inputs); }
+
hokein added inline comments.
Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12
LINK_LIBS
clangAST
mstorsjo wrote:
> Note that this file changed a bit further in
> 68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase
hokein updated this revision to Diff 476410.
hokein marked 2 inline comments as done.
hokein added a comment.
address comment: split tests, add a smoke test for
PragmaInclude::isSelfContained.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
kadircet added a comment.
thanks, mostly LG a bunch of suggestions for tests
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:74
+ /// Returns true if the given file is a self-contained file.
+ bool isSelfContained(const FileEntry *File)
mstorsjo added inline comments.
Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12
LINK_LIBS
clangAST
Note that this file changed a bit further in
68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase again.
(I tested
hokein marked 2 inline comments as done.
hokein added inline comments.
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39
FileID FID = SM.getFileID(Loc.physical());
-const auto *FE = SM.getFileEntryForID(FID);
+const auto *FE =
hokein updated this revision to Diff 476372.
hokein marked 4 inline comments as done.
hokein added a comment.
rebase and update based on the offline discussion
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39
FileID FID = SM.getFileID(Loc.physical());
-const auto *FE = SM.getFileEntryForID(FID);
+const auto *FE =
kadircet added inline comments.
Comment at:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:100
+ /// Contains all non self-contained files during the parsing.
+ llvm::DenseSet NonSelfContainedFiles;
s/during/detected during/
hokein updated this revision to Diff 475067.
hokein marked 3 inline comments as done.
hokein added a comment.
rebase and address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
https://reviews.llvm.org/D137698
Files:
kadircet added a comment.
i guess this patch needs a rebase for other pieces as well? but LG in general
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:47
+ const PragmaIncludes ) {
+ const FileEntry *FE =
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.
Based on https://reviews.llvm.org/D137697
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137698
15 matches
Mail list logo
