VitaNuo updated this revision to Diff 555985. VitaNuo added a comment. Fix analysis test to properly recognize the resource directory. Use HeaderSearch->getModuleMap().BuiltinDir to find out the resource directory.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157610/new/ https://reviews.llvm.org/D157610 Files: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp clang-tools-extra/clangd/IncludeCleaner.cpp clang-tools-extra/clangd/test/include-cleaner-batch-fix.test clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp clang-tools-extra/include-cleaner/lib/Analysis.cpp clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp =================================================================== --- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -273,6 +273,32 @@ EXPECT_THAT(Results.Unused, testing::IsEmpty()); } +TEST_F(AnalyzeTest, ResourceDirIsIgnored) { + Inputs.ExtraArgs.push_back("-resource-dir"); + Inputs.ExtraArgs.push_back("resources"); + Inputs.ExtraArgs.push_back("-internal-isystem"); + Inputs.ExtraArgs.push_back("resources/include"); + Inputs.Code = R"cpp( + #include <amintrin.h> + #include <imintrin.h> + void baz() { + bar(); + } + )cpp"; + Inputs.ExtraFiles["resources/include/amintrin.h"] = ""; + Inputs.ExtraFiles["resources/include/emintrin.h"] = guard(R"cpp( + void bar(); + )cpp"); + Inputs.ExtraFiles["resources/include/imintrin.h"] = guard(R"cpp( + #include <emintrin.h> + )cpp"); + TestAST AST(Inputs); + auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(), + AST.preprocessor().getHeaderSearchInfo()); + EXPECT_THAT(Results.Unused, testing::IsEmpty()); + EXPECT_THAT(Results.Missing, testing::IsEmpty()); +} + TEST(FixIncludes, Basic) { llvm::StringRef Code = R"cpp(#include "d.h" #include "a.h" Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp =================================================================== --- clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -13,6 +13,7 @@ #include "clang-include-cleaner/Types.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/Basic/DirectoryEntry.h" #include "clang/Basic/FileEntry.h" #include "clang/Basic/SourceManager.h" #include "clang/Format/Format.h" @@ -68,12 +69,17 @@ llvm::StringSet<> Missing; if (!HeaderFilter) HeaderFilter = [](llvm::StringRef) { return false; }; + const DirectoryEntry *ResourceDir = HS.getModuleMap().getBuiltinDir(); walkUsed(ASTRoots, MacroRefs, PI, SM, [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) { bool Satisfied = false; for (const Header &H : Providers) { - if (H.kind() == Header::Physical && H.physical() == MainFile) + if (H.kind() == Header::Physical && + (H.physical() == MainFile || + H.physical()->getDir() == ResourceDir)) { Satisfied = true; + continue; + } for (const Include *I : Inc.match(H)) { Used.insert(I); Satisfied = true; @@ -88,7 +94,8 @@ AnalysisResults Results; for (const Include &I : Inc.all()) { if (Used.contains(&I) || !I.Resolved || - HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName())) + HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()) || + I.Resolved->getFileEntry().getDir() == ResourceDir) continue; if (PI) { if (PI->shouldKeep(*I.Resolved)) Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -574,6 +574,29 @@ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); } +TEST(IncludeCleaner, ResourceDirIsIgnored) { + auto TU = TestTU::withCode(R"cpp( + #include <amintrin.h> + #include <imintrin.h> + void baz() { + bar(); + } + )cpp"); + TU.ExtraArgs.push_back("-resource-dir"); + TU.ExtraArgs.push_back(testPath("resources")); + TU.AdditionalFiles["resources/include/amintrin.h"] = ""; + TU.AdditionalFiles["resources/include/imintrin.h"] = guard(R"cpp( + #include <emintrin.h> + )cpp"); + TU.AdditionalFiles["resources/include/emintrin.h"] = guard(R"cpp( + void bar(); + )cpp"); + auto AST = TU.build(); + auto Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); + EXPECT_THAT(Findings.MissingIncludes, IsEmpty()); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test =================================================================== --- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test +++ clang-tools-extra/clangd/test/include-cleaner-batch-fix.test @@ -5,10 +5,11 @@ # RUN: rm -rf %t # RUN: mkdir -p %t/clangd # RUN: cp -r %S/Inputs/include-cleaner %t/include +# RUN: echo '-I%t/include' > %t/compile_flags.txt # Create a config file enabling include-cleaner features. # RUN: echo $'Diagnostics:\n UnusedIncludes: Strict\n MissingIncludes: Strict' >> %t/clangd/config.yaml -# RUN: env XDG_CONFIG_HOME=%t clangd -lit-test -enable-config --resource-dir=%t < %s | FileCheck -strict-whitespace %s +# RUN: env XDG_CONFIG_HOME=%t clangd -lit-test -enable-config --compile-commands-dir=%t < %s | FileCheck -strict-whitespace %s {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true, "changeAnnotationSupport":{"groupsOnLabel":true}}}},"trace":"off"}} --- { Index: clang-tools-extra/clangd/IncludeCleaner.cpp =================================================================== --- clang-tools-extra/clangd/IncludeCleaner.cpp +++ clang-tools-extra/clangd/IncludeCleaner.cpp @@ -75,6 +75,11 @@ auto FE = AST.getSourceManager().getFileManager().getFileRef( AST.getIncludeStructure().getRealPath(HID)); assert(FE); + if (FE->getDir() == AST.getPreprocessor() + .getHeaderSearchInfo() + .getModuleMap() + .getBuiltinDir()) + return false; if (PI && PI->shouldKeep(*FE)) return false; // FIXME(kirillbobyrev): We currently do not support the umbrella headers. @@ -392,6 +397,10 @@ std::vector<MissingIncludeDiagInfo> MissingIncludes; llvm::DenseSet<IncludeStructure::HeaderID> Used; trace::Span Tracer("include_cleaner::walkUsed"); + const DirectoryEntry *ResourceDir = AST.getPreprocessor() + .getHeaderSearchInfo() + .getModuleMap() + .getBuiltinDir(); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, AST.getPragmaIncludes().get(), SM, @@ -400,7 +409,8 @@ bool Satisfied = false; for (const auto &H : Providers) { if (H.kind() == include_cleaner::Header::Physical && - (H.physical() == MainFile || H.physical() == PreamblePatch)) { + (H.physical() == MainFile || H.physical() == PreamblePatch || + H.physical()->getLastRef().getDir() == ResourceDir)) { Satisfied = true; continue; } Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp +++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp @@ -26,6 +26,7 @@ #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Format/Format.h" +#include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Inclusions/HeaderIncludes.h" @@ -118,6 +119,7 @@ MainFileDecls.push_back(D); } llvm::DenseSet<include_cleaner::Symbol> SeenSymbols; + const DirectoryEntry *ResourceDir = HS->getModuleMap().getBuiltinDir(); // FIXME: Find a way to have less code duplication between include-cleaner // analysis implementation and the below code. walkUsed(MainFileDecls, RecordedPreprocessor.MacroReferences, &RecordedPI, @@ -140,8 +142,11 @@ bool Satisfied = false; for (const include_cleaner::Header &H : Providers) { if (H.kind() == include_cleaner::Header::Physical && - H.physical() == MainFile) + (H.physical() == MainFile || + H.physical()->getDir() == ResourceDir)) { Satisfied = true; + continue; + } for (const include_cleaner::Include *I : RecordedPreprocessor.Includes.match(H)) { @@ -158,7 +163,7 @@ std::vector<const include_cleaner::Include *> Unused; for (const include_cleaner::Include &I : RecordedPreprocessor.Includes.all()) { - if (Used.contains(&I) || !I.Resolved) + if (Used.contains(&I) || !I.Resolved || I.Resolved->getDir() == ResourceDir) continue; if (RecordedPI.shouldKeep(*I.Resolved)) continue;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits