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 &PI) {
+ const FileEntry *FE = SM.getFileEntryForID(FID);
+ for (; FID != SM.getMainFileID(); FE = SM.getFileEntryForID(FID)) {
----------------
with the API i suggested in the base revision this should look like:
```
while(FID != SM.getMainFileID() && !PI.isSelfContained(FID))
FID = SM.getFileID(SM.getIncludeLoc(FID));
return SM.getFileEntryForID(FID);
```
i think feels more natural
================
Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:53
+ Out->NonSelfContainedFiles.erase(
+ SM.getFileEntryForID(PrevFID)->getUniqueID());
+ else
----------------
`FE->getUniqueID()`, same below
================
Comment at: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp:149
+ Inputs.ExtraFiles["detail2.h"] = guard("");
+ Inputs.ExtraFiles["imp.inc"] = "";
TestAST AST(Inputs);
----------------
nit: rather than introducing a 3rd header, i'd just include it in `header2.h`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137698/new/
https://reviews.llvm.org/D137698
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
