[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 10 08:34:35 PDT 2023


kadircet added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:122
   llvm::DenseSet<include_cleaner::Symbol> SeenSymbols;
+  std::string ResourceDir = HS->getHeaderSearchOpts().ResourceDir;
   // FIXME: Find a way to have less code duplication between include-cleaner
----------------
let's use `HS->getModuleMap().getBuiltinDir()` then we can get away with just comparing that pointer to `H.physical()->getLastRef().getDir()` (same applies to all the other places as well)


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:309
       continue;
+    auto Dir = llvm::StringRef{MFI.Resolved}.rsplit('/').first;
+    if (Dir == AST.getPreprocessor()
----------------
let's move this into `mayConsiderUnused`, we also convert this include into a FileEntry in there, so we can directly compare the directory agian.


================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:579-580
+  auto TU = TestTU::withCode(R"cpp(
+    #include "resources/amintrin.h"
+    #include "resources/imintrin.h"
+    void baz() {
----------------
can you rather include these as `<amintrin.h>`


================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:587-588
+  TU.ExtraArgs.push_back(testPath("resources"));
+  TU.AdditionalFiles["resources/amintrin.h"] = "";
+   TU.AdditionalFiles["resources/imintrin.h"] = guard(R"cpp(
+    #include "emintrin.h"
----------------
we should put these under `resources/include/xxxx.h`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157610



More information about the cfe-commits mailing list