[clang-tools-extra] 43c2036 - [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

Viktoriia Bakalova via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 7 04:39:27 PDT 2023


Author: Viktoriia Bakalova
Date: 2023-09-07T11:39:18Z
New Revision: 43c20367f417410a736959d4ae53f374e0d5b500

URL: https://github.com/llvm/llvm-project/commit/43c20367f417410a736959d4ae53f374e0d5b500
DIFF: https://github.com/llvm/llvm-project/commit/43c20367f417410a736959d4ae53f374e0d5b500.diff

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

Differential Revision: https://reviews.llvm.org/D157610

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index 8d5f400acfef8bc..8e460cb38826eb1 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/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"
@@ -119,6 +120,8 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
     MainFileDecls.push_back(D);
   }
   llvm::DenseSet<include_cleaner::Symbol> SeenSymbols;
+  const DirectoryEntry *ResourceDir =
+      PP->getHeaderSearchInfo().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,
@@ -141,8 +144,11 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
              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)) {
@@ -159,7 +165,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
   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;

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 1fcb5c7228fb639..a6e01eb72821d22 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -75,6 +75,11 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
   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 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
   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(), AST.getPreprocessor(),
@@ -400,7 +409,8 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
         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;
           }

diff  --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
index 72d69ec0d494156..af7cdba5b05f431 100644
--- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ b/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"}}
 ---
 {

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 1d6b99af0814292..48c9c3f576640dc 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -574,6 +574,29 @@ TEST(IncludeCleaner, VerbatimEquivalence) {
   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"] = guard("");
+  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

diff  --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 630be33e7916903..8c6a23d283d3a09 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/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"
@@ -86,12 +87,17 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
   llvm::StringSet<> Missing;
   if (!HeaderFilter)
     HeaderFilter = [](llvm::StringRef) { return false; };
+  const DirectoryEntry *ResourceDir =
+      PP.getHeaderSearchInfo().getModuleMap().getBuiltinDir();
   walkUsed(ASTRoots, MacroRefs, PI, PP,
            [&](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;
+               }
                for (const Include *I : Inc.match(H)) {
                  Used.insert(I);
                  Satisfied = true;
@@ -107,7 +113,8 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
   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))

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index daa5f91463ec271..0b03c643a94c1ab 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -271,6 +271,31 @@ TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
   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"] = guard("");
+  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.preprocessor());
+  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"


        


More information about the cfe-commits mailing list