Skip to content

Commit 7943169

Browse files
sam-mccallsam-mccall
committed
[clang-include-cleaner] make SymbolLocation a real class, move FindHeaders
- replace SymbolLocation std::variant with enum-exposing version similar to those in types.cpp. There's no appropriate implementation file, added LocateSymbol.cpp in anticipation of locateDecl/locateMacro. - FindHeaders is not part of the public Analysis interface, so should not be implemented/tested there (just code organization) - rename findIncludeHeaders->findHeaders to avoid confusion with Include concept Differential Revision: https://reviews.llvm.org/D137825
1 parent 1da74ee commit 7943169

File tree

8 files changed

+196
-101
lines changed

8 files changed

+196
-101
lines changed

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

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -28,52 +28,18 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
2828
if (auto SS = Recognizer(&ND)) {
2929
// FIXME: Also report forward decls from main-file, so that the caller
3030
// can decide to insert/ignore a header.
31-
return CB({Loc, Symbol(*SS), RT}, findIncludeHeaders(*SS, SM, PI));
31+
return CB({Loc, Symbol(*SS), RT}, findHeaders(*SS, SM, PI));
3232
}
3333
// FIXME: Extract locations from redecls.
34-
return CB({Loc, Symbol(ND), RT},
35-
findIncludeHeaders(ND.getLocation(), SM, PI));
34+
return CB({Loc, Symbol(ND), RT}, findHeaders(ND.getLocation(), SM, PI));
3635
});
3736
}
3837
for (const SymbolReference &MacroRef : MacroRefs) {
3938
assert(MacroRef.Target.kind() == Symbol::Macro);
4039
// FIXME: Handle macro locations.
4140
return CB(MacroRef,
42-
findIncludeHeaders(MacroRef.Target.macro().Definition, SM, PI));
41+
findHeaders(MacroRef.Target.macro().Definition, SM, PI));
4342
}
4443
}
4544

46-
llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &SLoc,
47-
const SourceManager &SM,
48-
const PragmaIncludes &PI) {
49-
llvm::SmallVector<Header> Results;
50-
if (auto *Loc = std::get_if<SourceLocation>(&SLoc)) {
51-
// FIXME: Handle non self-contained files.
52-
FileID FID = SM.getFileID(*Loc);
53-
const auto *FE = SM.getFileEntryForID(FID);
54-
if (!FE)
55-
return {};
56-
57-
// We treat the spelling header in the IWYU pragma as the final public
58-
// header.
59-
// FIXME: look for exporters if the public header is exported by another
60-
// header.
61-
llvm::StringRef VerbatimSpelling = PI.getPublic(FE);
62-
if (!VerbatimSpelling.empty())
63-
return {Header(VerbatimSpelling)};
64-
65-
Results = {Header(FE)};
66-
// FIXME: compute transitive exporter headers.
67-
for (const auto *Export : PI.getExporters(FE, SM.getFileManager()))
68-
Results.push_back(Export);
69-
return Results;
70-
}
71-
if (auto *Sym = std::get_if<tooling::stdlib::Symbol>(&SLoc)) {
72-
for (const auto &H : Sym->headers())
73-
Results.push_back(H);
74-
return Results;
75-
}
76-
llvm_unreachable("unhandled SymbolLocation kind!");
77-
}
78-
7945
} // namespace clang::include_cleaner

‎clang-tools-extra/include-cleaner/lib/AnalysisInternal.h

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,41 @@ namespace include_cleaner {
4747
void walkAST(Decl &Root,
4848
llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>);
4949

50-
/// A location where a symbol can be provided.
50+
/// A place where a symbol can be provided.
5151
/// It is either a physical file of the TU (SourceLocation) or a logical
5252
/// location in the standard library (stdlib::Symbol).
53-
// FIXME: use a real Class!
54-
using SymbolLocation = std::variant<SourceLocation, tooling::stdlib::Symbol>;
53+
struct SymbolLocation {
54+
enum Kind {
55+
/// A position within a source file (or macro expansion) parsed by clang.
56+
Physical,
57+
/// A recognized standard library symbol, like std::string.
58+
Standard,
59+
};
60+
61+
SymbolLocation(SourceLocation S) : Storage(S) {}
62+
SymbolLocation(tooling::stdlib::Symbol S) : Storage(S) {}
63+
64+
Kind kind() const { return static_cast<Kind>(Storage.index()); }
65+
bool operator==(const SymbolLocation &RHS) const {
66+
return Storage == RHS.Storage;
67+
}
68+
69+
SourceLocation physical() const { return std::get<Physical>(Storage); }
70+
tooling::stdlib::Symbol standard() const {
71+
return std::get<Standard>(Storage);
72+
}
73+
74+
private:
75+
// Order must match Kind enum!
76+
std::variant<SourceLocation, tooling::stdlib::Symbol> Storage;
77+
};
78+
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Header &);
5579

5680
/// Finds the headers that provide the symbol location.
5781
// FIXME: expose signals
58-
llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &Loc,
59-
const SourceManager &SM,
60-
const PragmaIncludes &PI);
82+
llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
83+
const SourceManager &SM,
84+
const PragmaIncludes &PI);
6185

6286
/// Write an HTML summary of the analysis to the given stream.
6387
/// FIXME: Once analysis has a public API, this should be public too.

‎clang-tools-extra/include-cleaner/lib/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ set(LLVM_LINK_COMPONENTS Support)
22

33
add_clang_library(clangIncludeCleaner
44
Analysis.cpp
5+
FindHeaders.cpp
56
HTMLReport.cpp
7+
LocateSymbol.cpp
68
Record.cpp
79
Types.cpp
810
WalkAST.cpp

‎clang-tools-extra/include-cleaner/lib/FindHeaders.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//===--- FindHeaders.cpp --------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "AnalysisInternal.h"
10+
#include "clang-include-cleaner/Record.h"
11+
#include "clang/Basic/SourceManager.h"
12+
13+
namespace clang::include_cleaner {
14+
15+
llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
16+
const SourceManager &SM,
17+
const PragmaIncludes &PI) {
18+
llvm::SmallVector<Header> Results;
19+
switch (Loc.kind()) {
20+
case SymbolLocation::Physical: {
21+
// FIXME: Handle non self-contained files.
22+
FileID FID = SM.getFileID(Loc.physical());
23+
const auto *FE = SM.getFileEntryForID(FID);
24+
if (!FE)
25+
return {};
26+
27+
// We treat the spelling header in the IWYU pragma as the final public
28+
// header.
29+
// FIXME: look for exporters if the public header is exported by another
30+
// header.
31+
llvm::StringRef VerbatimSpelling = PI.getPublic(FE);
32+
if (!VerbatimSpelling.empty())
33+
return {Header(VerbatimSpelling)};
34+
35+
Results = {Header(FE)};
36+
// FIXME: compute transitive exporter headers.
37+
for (const auto *Export : PI.getExporters(FE, SM.getFileManager()))
38+
Results.push_back(Export);
39+
return Results;
40+
}
41+
case SymbolLocation::Standard: {
42+
for (const auto &H : Loc.standard().headers())
43+
Results.push_back(H);
44+
return Results;
45+
}
46+
}
47+
llvm_unreachable("unhandled SymbolLocation kind!");
48+
}
49+
50+
} // namespace clang::include_cleaner

‎clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//===--- LocateSymbol.cpp -------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "AnalysisInternal.h"
10+
#include "llvm/ADT/StringExtras.h"
11+
#include "llvm/Support/raw_ostream.h"
12+
13+
namespace clang::include_cleaner {
14+
15+
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolLocation &S) {
16+
switch (S.kind()) {
17+
case SymbolLocation::Physical:
18+
// We can't decode the Location without SourceManager. Its raw
19+
// representation isn't completely useless (and distinguishes
20+
// SymbolReference from Symbol).
21+
return OS << "@0x"
22+
<< llvm::utohexstr(
23+
S.physical().getRawEncoding(), /*LowerCase=*/false,
24+
/*Width=*/CHAR_BIT * sizeof(SourceLocation::UIntTy));
25+
case SymbolLocation::Standard:
26+
return OS << S.standard().scope() << S.standard().name();
27+
}
28+
llvm_unreachable("Unhandled Symbol kind");
29+
}
30+
31+
} // namespace clang::include_cleaner

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

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang-include-cleaner/Analysis.h"
10-
#include "AnalysisInternal.h"
1110
#include "clang-include-cleaner/Record.h"
1211
#include "clang-include-cleaner/Types.h"
1312
#include "clang/AST/ASTContext.h"
@@ -130,63 +129,6 @@ TEST(WalkUsed, MacroRefs) {
130129
UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile))));
131130
}
132131

133-
TEST(FindIncludeHeaders, IWYU) {
134-
TestInputs Inputs;
135-
PragmaIncludes PI;
136-
Inputs.MakeAction = [&PI] {
137-
struct Hook : public PreprocessOnlyAction {
138-
public:
139-
Hook(PragmaIncludes *Out) : Out(Out) {}
140-
bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
141-
Out->record(CI);
142-
return true;
143-
}
144-
145-
PragmaIncludes *Out;
146-
};
147-
return std::make_unique<Hook>(&PI);
148-
};
149-
150-
Inputs.Code = R"cpp(
151-
#include "header1.h"
152-
#include "header2.h"
153-
)cpp";
154-
Inputs.ExtraFiles["header1.h"] = R"cpp(
155-
// IWYU pragma: private, include "path/public.h"
156-
)cpp";
157-
Inputs.ExtraFiles["header2.h"] = R"cpp(
158-
#include "detail1.h" // IWYU pragma: export
159-
160-
// IWYU pragma: begin_exports
161-
#include "detail2.h"
162-
// IWYU pragma: end_exports
163-
164-
#include "normal.h"
165-
)cpp";
166-
Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
167-
Inputs.ExtraFiles["detail2.h"] = "";
168-
TestAST AST(Inputs);
169-
const auto &SM = AST.sourceManager();
170-
auto &FM = SM.getFileManager();
171-
// Returns the source location for the start of the file.
172-
auto SourceLocFromFile = [&](llvm::StringRef FileName) {
173-
return SM.translateFileLineCol(FM.getFile(FileName).get(),
174-
/*Line=*/1, /*Col=*/1);
175-
};
176-
177-
EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("header1.h"), SM, PI),
178-
UnorderedElementsAre(Header("\"path/public.h\"")));
179-
180-
EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail1.h"), SM, PI),
181-
UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
182-
Header(FM.getFile("detail1.h").get())));
183-
EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail2.h"), SM, PI),
184-
UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
185-
Header(FM.getFile("detail2.h").get())));
186-
187-
EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("normal.h"), SM, PI),
188-
UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
189-
}
190132

191133
} // namespace
192134
} // namespace clang::include_cleaner

‎clang-tools-extra/include-cleaner/unittests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
66
add_custom_target(ClangIncludeCleanerUnitTests)
77
add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
88
AnalysisTest.cpp
9+
FindHeadersTest.cpp
910
RecordTest.cpp
1011
WalkASTTest.cpp
1112
)

‎clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
//===--- FindHeadersTest.cpp ----------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "AnalysisInternal.h"
10+
#include "clang-include-cleaner/Record.h"
11+
#include "clang/Frontend/FrontendActions.h"
12+
#include "clang/Testing/TestAST.h"
13+
#include "gmock/gmock.h"
14+
#include "gtest/gtest.h"
15+
16+
namespace clang::include_cleaner {
17+
namespace {
18+
using testing::UnorderedElementsAre;
19+
20+
TEST(FindIncludeHeaders, IWYU) {
21+
TestInputs Inputs;
22+
PragmaIncludes PI;
23+
Inputs.MakeAction = [&PI] {
24+
struct Hook : public PreprocessOnlyAction {
25+
public:
26+
Hook(PragmaIncludes *Out) : Out(Out) {}
27+
bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
28+
Out->record(CI);
29+
return true;
30+
}
31+
32+
PragmaIncludes *Out;
33+
};
34+
return std::make_unique<Hook>(&PI);
35+
};
36+
37+
Inputs.Code = R"cpp(
38+
#include "header1.h"
39+
#include "header2.h"
40+
)cpp";
41+
Inputs.ExtraFiles["header1.h"] = R"cpp(
42+
// IWYU pragma: private, include "path/public.h"
43+
)cpp";
44+
Inputs.ExtraFiles["header2.h"] = R"cpp(
45+
#include "detail1.h" // IWYU pragma: export
46+
47+
// IWYU pragma: begin_exports
48+
#include "detail2.h"
49+
// IWYU pragma: end_exports
50+
51+
#include "normal.h"
52+
)cpp";
53+
Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
54+
Inputs.ExtraFiles["detail2.h"] = "";
55+
TestAST AST(Inputs);
56+
const auto &SM = AST.sourceManager();
57+
auto &FM = SM.getFileManager();
58+
// Returns the source location for the start of the file.
59+
auto SourceLocFromFile = [&](llvm::StringRef FileName) {
60+
return SM.translateFileLineCol(FM.getFile(FileName).get(),
61+
/*Line=*/1, /*Col=*/1);
62+
};
63+
64+
EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, PI),
65+
UnorderedElementsAre(Header("\"path/public.h\"")));
66+
67+
EXPECT_THAT(findHeaders(SourceLocFromFile("detail1.h"), SM, PI),
68+
UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
69+
Header(FM.getFile("detail1.h").get())));
70+
EXPECT_THAT(findHeaders(SourceLocFromFile("detail2.h"), SM, PI),
71+
UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
72+
Header(FM.getFile("detail2.h").get())));
73+
74+
EXPECT_THAT(findHeaders(SourceLocFromFile("normal.h"), SM, PI),
75+
UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
76+
}
77+
78+
} // namespace
79+
} // namespace clang::include_cleaner

0 commit comments

Comments
 (0)