[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 22 06:46:51 PDT 2023


kadircet added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp:54
+    "ignore-headers",
+    cl::desc("A comma-separated list of headers to ignore."),
+    cl::init(""),
----------------
`A comma-separated list of regexes to match against suffix of a header, and disable analysis if matched.`


================
Comment at: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp:139
+                                      HeaderPattern, RegexError);
+        return;
+      }
----------------
erroring out only after we performed all of our analysis and build a TU feels unfortunate. can we build these regexes and verify them at main and pass onto action instead? (in case the action is run on multiple files at once, we shouldn't compile/verify regexes over and over again)


================
Comment at: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp:160
     auto Results =
         analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS);
     if (!Insert)
----------------
we actually want to perform filtering inside `analyze`. it's not enough to filter only when printing, we should also filter before applying fixes :D. but we should also be applying filtering to absolute paths in all cases, for missing includes this is being applied to spelling instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153340/new/

https://reviews.llvm.org/D153340



More information about the cfe-commits mailing list