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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits