[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 10 07:47:26 PST 2022


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



More information about the cfe-commits mailing list