Skip to content

Commit 43c2036

Browse files
committed
[include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.
Differential Revision: https://reviews.llvm.org/D157610
1 parent 9978934 commit 43c2036

File tree

6 files changed

+78
-6
lines changed

6 files changed

+78
-6
lines changed

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "clang/Basic/LangOptions.h"
2727
#include "clang/Basic/SourceLocation.h"
2828
#include "clang/Format/Format.h"
29+
#include "clang/Lex/HeaderSearchOptions.h"
2930
#include "clang/Lex/Preprocessor.h"
3031
#include "clang/Tooling/Core/Replacement.h"
3132
#include "clang/Tooling/Inclusions/HeaderIncludes.h"
@@ -119,6 +120,8 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
119120
MainFileDecls.push_back(D);
120121
}
121122
llvm::DenseSet<include_cleaner::Symbol> SeenSymbols;
123+
const DirectoryEntry *ResourceDir =
124+
PP->getHeaderSearchInfo().getModuleMap().getBuiltinDir();
122125
// FIXME: Find a way to have less code duplication between include-cleaner
123126
// analysis implementation and the below code.
124127
walkUsed(MainFileDecls, RecordedPreprocessor.MacroReferences, &RecordedPI,
@@ -141,8 +144,11 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
141144
bool Satisfied = false;
142145
for (const include_cleaner::Header &H : Providers) {
143146
if (H.kind() == include_cleaner::Header::Physical &&
144-
H.physical() == MainFile)
147+
(H.physical() == MainFile ||
148+
H.physical()->getDir() == ResourceDir)) {
145149
Satisfied = true;
150+
continue;
151+
}
146152

147153
for (const include_cleaner::Include *I :
148154
RecordedPreprocessor.Includes.match(H)) {
@@ -159,7 +165,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
159165
std::vector<const include_cleaner::Include *> Unused;
160166
for (const include_cleaner::Include &I :
161167
RecordedPreprocessor.Includes.all()) {
162-
if (Used.contains(&I) || !I.Resolved)
168+
if (Used.contains(&I) || !I.Resolved || I.Resolved->getDir() == ResourceDir)
163169
continue;
164170
if (RecordedPI.shouldKeep(*I.Resolved))
165171
continue;

clang-tools-extra/clangd/IncludeCleaner.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
7575
auto FE = AST.getSourceManager().getFileManager().getFileRef(
7676
AST.getIncludeStructure().getRealPath(HID));
7777
assert(FE);
78+
if (FE->getDir() == AST.getPreprocessor()
79+
.getHeaderSearchInfo()
80+
.getModuleMap()
81+
.getBuiltinDir())
82+
return false;
7883
if (PI && PI->shouldKeep(*FE))
7984
return false;
8085
// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
@@ -392,6 +397,10 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
392397
std::vector<MissingIncludeDiagInfo> MissingIncludes;
393398
llvm::DenseSet<IncludeStructure::HeaderID> Used;
394399
trace::Span Tracer("include_cleaner::walkUsed");
400+
const DirectoryEntry *ResourceDir = AST.getPreprocessor()
401+
.getHeaderSearchInfo()
402+
.getModuleMap()
403+
.getBuiltinDir();
395404
include_cleaner::walkUsed(
396405
AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
397406
AST.getPragmaIncludes().get(), AST.getPreprocessor(),
@@ -400,7 +409,8 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
400409
bool Satisfied = false;
401410
for (const auto &H : Providers) {
402411
if (H.kind() == include_cleaner::Header::Physical &&
403-
(H.physical() == MainFile || H.physical() == PreamblePatch)) {
412+
(H.physical() == MainFile || H.physical() == PreamblePatch ||
413+
H.physical()->getLastRef().getDir() == ResourceDir)) {
404414
Satisfied = true;
405415
continue;
406416
}

clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
# RUN: rm -rf %t
66
# RUN: mkdir -p %t/clangd
77
# RUN: cp -r %S/Inputs/include-cleaner %t/include
8+
# RUN: echo '-I%t/include' > %t/compile_flags.txt
89
# Create a config file enabling include-cleaner features.
910
# RUN: echo $'Diagnostics:\n UnusedIncludes: Strict\n MissingIncludes: Strict' >> %t/clangd/config.yaml
1011

11-
# RUN: env XDG_CONFIG_HOME=%t clangd -lit-test -enable-config --resource-dir=%t < %s | FileCheck -strict-whitespace %s
12+
# RUN: env XDG_CONFIG_HOME=%t clangd -lit-test -enable-config --compile-commands-dir=%t < %s | FileCheck -strict-whitespace %s
1213
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true, "changeAnnotationSupport":{"groupsOnLabel":true}}}},"trace":"off"}}
1314
---
1415
{

clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,29 @@ TEST(IncludeCleaner, VerbatimEquivalence) {
574574
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
575575
}
576576

577+
TEST(IncludeCleaner, ResourceDirIsIgnored) {
578+
auto TU = TestTU::withCode(R"cpp(
579+
#include <amintrin.h>
580+
#include <imintrin.h>
581+
void baz() {
582+
bar();
583+
}
584+
)cpp");
585+
TU.ExtraArgs.push_back("-resource-dir");
586+
TU.ExtraArgs.push_back(testPath("resources"));
587+
TU.AdditionalFiles["resources/include/amintrin.h"] = guard("");
588+
TU.AdditionalFiles["resources/include/imintrin.h"] = guard(R"cpp(
589+
#include <emintrin.h>
590+
)cpp");
591+
TU.AdditionalFiles["resources/include/emintrin.h"] = guard(R"cpp(
592+
void bar();
593+
)cpp");
594+
auto AST = TU.build();
595+
auto Findings = computeIncludeCleanerFindings(AST);
596+
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
597+
EXPECT_THAT(Findings.MissingIncludes, IsEmpty());
598+
}
599+
577600
} // namespace
578601
} // namespace clangd
579602
} // namespace clang

clang-tools-extra/include-cleaner/lib/Analysis.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "clang-include-cleaner/Types.h"
1414
#include "clang/AST/Decl.h"
1515
#include "clang/AST/DeclBase.h"
16+
#include "clang/Basic/DirectoryEntry.h"
1617
#include "clang/Basic/FileEntry.h"
1718
#include "clang/Basic/SourceManager.h"
1819
#include "clang/Format/Format.h"
@@ -86,12 +87,17 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
8687
llvm::StringSet<> Missing;
8788
if (!HeaderFilter)
8889
HeaderFilter = [](llvm::StringRef) { return false; };
90+
const DirectoryEntry *ResourceDir =
91+
PP.getHeaderSearchInfo().getModuleMap().getBuiltinDir();
8992
walkUsed(ASTRoots, MacroRefs, PI, PP,
9093
[&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
9194
bool Satisfied = false;
9295
for (const Header &H : Providers) {
93-
if (H.kind() == Header::Physical && H.physical() == MainFile)
96+
if (H.kind() == Header::Physical &&
97+
(H.physical() == MainFile ||
98+
H.physical()->getDir() == ResourceDir)) {
9499
Satisfied = true;
100+
}
95101
for (const Include *I : Inc.match(H)) {
96102
Used.insert(I);
97103
Satisfied = true;
@@ -107,7 +113,8 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
107113
AnalysisResults Results;
108114
for (const Include &I : Inc.all()) {
109115
if (Used.contains(&I) || !I.Resolved ||
110-
HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()))
116+
HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()) ||
117+
I.Resolved->getFileEntry().getDir() == ResourceDir)
111118
continue;
112119
if (PI) {
113120
if (PI->shouldKeep(*I.Resolved))

clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,31 @@ TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
271271
EXPECT_THAT(Results.Unused, testing::IsEmpty());
272272
}
273273

274+
TEST_F(AnalyzeTest, ResourceDirIsIgnored) {
275+
Inputs.ExtraArgs.push_back("-resource-dir");
276+
Inputs.ExtraArgs.push_back("resources");
277+
Inputs.ExtraArgs.push_back("-internal-isystem");
278+
Inputs.ExtraArgs.push_back("resources/include");
279+
Inputs.Code = R"cpp(
280+
#include <amintrin.h>
281+
#include <imintrin.h>
282+
void baz() {
283+
bar();
284+
}
285+
)cpp";
286+
Inputs.ExtraFiles["resources/include/amintrin.h"] = guard("");
287+
Inputs.ExtraFiles["resources/include/emintrin.h"] = guard(R"cpp(
288+
void bar();
289+
)cpp");
290+
Inputs.ExtraFiles["resources/include/imintrin.h"] = guard(R"cpp(
291+
#include <emintrin.h>
292+
)cpp");
293+
TestAST AST(Inputs);
294+
auto Results = analyze({}, {}, PP.Includes, &PI, AST.preprocessor());
295+
EXPECT_THAT(Results.Unused, testing::IsEmpty());
296+
EXPECT_THAT(Results.Missing, testing::IsEmpty());
297+
}
298+
274299
TEST(FixIncludes, Basic) {
275300
llvm::StringRef Code = R"cpp(#include "d.h"
276301
#include "a.h"

0 commit comments

Comments
 (0)