[clang-tools-extra] 7943169 - [clang-include-cleaner] make SymbolLocation a real class, move FindHeaders
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 11 04:42:58 PST 2022
Author: Sam McCall
Date: 2022-11-11T13:41:54+01:00
New Revision: 7943169273b22f61a0322cb2d38ff75845e52628
URL: https://github.com/llvm/llvm-project/commit/7943169273b22f61a0322cb2d38ff75845e52628
DIFF: https://github.com/llvm/llvm-project/commit/7943169273b22f61a0322cb2d38ff75845e52628.diff
LOG: [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
Added:
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
Modified:
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index b3238e93bcc9..d20e5b162907 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -28,52 +28,18 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
if (auto SS = Recognizer(&ND)) {
// FIXME: Also report forward decls from main-file, so that the caller
// can decide to insert/ignore a header.
- return CB({Loc, Symbol(*SS), RT}, findIncludeHeaders(*SS, SM, PI));
+ return CB({Loc, Symbol(*SS), RT}, findHeaders(*SS, SM, PI));
}
// FIXME: Extract locations from redecls.
- return CB({Loc, Symbol(ND), RT},
- findIncludeHeaders(ND.getLocation(), SM, PI));
+ return CB({Loc, Symbol(ND), RT}, findHeaders(ND.getLocation(), SM, PI));
});
}
for (const SymbolReference &MacroRef : MacroRefs) {
assert(MacroRef.Target.kind() == Symbol::Macro);
// FIXME: Handle macro locations.
return CB(MacroRef,
- findIncludeHeaders(MacroRef.Target.macro().Definition, SM, PI));
+ findHeaders(MacroRef.Target.macro().Definition, SM, PI));
}
}
-llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &SLoc,
- const SourceManager &SM,
- const PragmaIncludes &PI) {
- llvm::SmallVector<Header> Results;
- if (auto *Loc = std::get_if<SourceLocation>(&SLoc)) {
- // FIXME: Handle non self-contained files.
- FileID FID = SM.getFileID(*Loc);
- const auto *FE = SM.getFileEntryForID(FID);
- if (!FE)
- return {};
-
- // We treat the spelling header in the IWYU pragma as the final public
- // header.
- // FIXME: look for exporters if the public header is exported by another
- // header.
- llvm::StringRef VerbatimSpelling = PI.getPublic(FE);
- if (!VerbatimSpelling.empty())
- return {Header(VerbatimSpelling)};
-
- Results = {Header(FE)};
- // FIXME: compute transitive exporter headers.
- for (const auto *Export : PI.getExporters(FE, SM.getFileManager()))
- Results.push_back(Export);
- return Results;
- }
- if (auto *Sym = std::get_if<tooling::stdlib::Symbol>(&SLoc)) {
- for (const auto &H : Sym->headers())
- Results.push_back(H);
- return Results;
- }
- llvm_unreachable("unhandled SymbolLocation kind!");
-}
-
} // namespace clang::include_cleaner
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index 18bcedde1a2e..17c11c1218dd 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -47,17 +47,41 @@ namespace include_cleaner {
void walkAST(Decl &Root,
llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>);
-/// A location where a symbol can be provided.
+/// A place where a symbol can be provided.
/// It is either a physical file of the TU (SourceLocation) or a logical
/// location in the standard library (stdlib::Symbol).
-// FIXME: use a real Class!
-using SymbolLocation = std::variant<SourceLocation, tooling::stdlib::Symbol>;
+struct SymbolLocation {
+ enum Kind {
+ /// A position within a source file (or macro expansion) parsed by clang.
+ Physical,
+ /// A recognized standard library symbol, like std::string.
+ Standard,
+ };
+
+ SymbolLocation(SourceLocation S) : Storage(S) {}
+ SymbolLocation(tooling::stdlib::Symbol S) : Storage(S) {}
+
+ Kind kind() const { return static_cast<Kind>(Storage.index()); }
+ bool operator==(const SymbolLocation &RHS) const {
+ return Storage == RHS.Storage;
+ }
+
+ SourceLocation physical() const { return std::get<Physical>(Storage); }
+ tooling::stdlib::Symbol standard() const {
+ return std::get<Standard>(Storage);
+ }
+
+private:
+ // Order must match Kind enum!
+ std::variant<SourceLocation, tooling::stdlib::Symbol> Storage;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Header &);
/// Finds the headers that provide the symbol location.
// FIXME: expose signals
-llvm::SmallVector<Header> findIncludeHeaders(const SymbolLocation &Loc,
- const SourceManager &SM,
- const PragmaIncludes &PI);
+llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
+ const SourceManager &SM,
+ const PragmaIncludes &PI);
/// Write an HTML summary of the analysis to the given stream.
/// FIXME: Once analysis has a public API, this should be public too.
diff --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index 62c61f44a0fa..617539ae0dba 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -2,7 +2,9 @@ set(LLVM_LINK_COMPONENTS Support)
add_clang_library(clangIncludeCleaner
Analysis.cpp
+ FindHeaders.cpp
HTMLReport.cpp
+ LocateSymbol.cpp
Record.cpp
Types.cpp
WalkAST.cpp
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
new file mode 100644
index 000000000000..e9e5126da114
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -0,0 +1,50 @@
+//===--- FindHeaders.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang::include_cleaner {
+
+llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
+ const SourceManager &SM,
+ const PragmaIncludes &PI) {
+ llvm::SmallVector<Header> Results;
+ switch (Loc.kind()) {
+ case SymbolLocation::Physical: {
+ // FIXME: Handle non self-contained files.
+ FileID FID = SM.getFileID(Loc.physical());
+ const auto *FE = SM.getFileEntryForID(FID);
+ if (!FE)
+ return {};
+
+ // We treat the spelling header in the IWYU pragma as the final public
+ // header.
+ // FIXME: look for exporters if the public header is exported by another
+ // header.
+ llvm::StringRef VerbatimSpelling = PI.getPublic(FE);
+ if (!VerbatimSpelling.empty())
+ return {Header(VerbatimSpelling)};
+
+ Results = {Header(FE)};
+ // FIXME: compute transitive exporter headers.
+ for (const auto *Export : PI.getExporters(FE, SM.getFileManager()))
+ Results.push_back(Export);
+ return Results;
+ }
+ case SymbolLocation::Standard: {
+ for (const auto &H : Loc.standard().headers())
+ Results.push_back(H);
+ return Results;
+ }
+ }
+ llvm_unreachable("unhandled SymbolLocation kind!");
+}
+
+} // namespace clang::include_cleaner
\ No newline at end of file
diff --git a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
new file mode 100644
index 000000000000..03c32e656ee4
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -0,0 +1,31 @@
+//===--- LocateSymbol.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AnalysisInternal.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang::include_cleaner {
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolLocation &S) {
+ switch (S.kind()) {
+ case SymbolLocation::Physical:
+ // We can't decode the Location without SourceManager. Its raw
+ // representation isn't completely useless (and distinguishes
+ // SymbolReference from Symbol).
+ return OS << "@0x"
+ << llvm::utohexstr(
+ S.physical().getRawEncoding(), /*LowerCase=*/false,
+ /*Width=*/CHAR_BIT * sizeof(SourceLocation::UIntTy));
+ case SymbolLocation::Standard:
+ return OS << S.standard().scope() << S.standard().name();
+ }
+ llvm_unreachable("Unhandled Symbol kind");
+}
+
+} // namespace clang::include_cleaner
\ No newline at end of file
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 594a0215c95a..6d9237c6b7d7 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "clang-include-cleaner/Analysis.h"
-#include "AnalysisInternal.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
@@ -130,63 +129,6 @@ TEST(WalkUsed, MacroRefs) {
UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile))));
}
-TEST(FindIncludeHeaders, IWYU) {
- TestInputs Inputs;
- PragmaIncludes PI;
- Inputs.MakeAction = [&PI] {
- struct Hook : public PreprocessOnlyAction {
- public:
- Hook(PragmaIncludes *Out) : Out(Out) {}
- bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
- Out->record(CI);
- return true;
- }
-
- PragmaIncludes *Out;
- };
- return std::make_unique<Hook>(&PI);
- };
-
- Inputs.Code = R"cpp(
- #include "header1.h"
- #include "header2.h"
- )cpp";
- Inputs.ExtraFiles["header1.h"] = R"cpp(
- // IWYU pragma: private, include "path/public.h"
- )cpp";
- Inputs.ExtraFiles["header2.h"] = R"cpp(
- #include "detail1.h" // IWYU pragma: export
-
- // IWYU pragma: begin_exports
- #include "detail2.h"
- // IWYU pragma: end_exports
-
- #include "normal.h"
- )cpp";
- Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
- Inputs.ExtraFiles["detail2.h"] = "";
- TestAST AST(Inputs);
- const auto &SM = AST.sourceManager();
- auto &FM = SM.getFileManager();
- // Returns the source location for the start of the file.
- auto SourceLocFromFile = [&](llvm::StringRef FileName) {
- return SM.translateFileLineCol(FM.getFile(FileName).get(),
- /*Line=*/1, /*Col=*/1);
- };
-
- EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("header1.h"), SM, PI),
- UnorderedElementsAre(Header("\"path/public.h\"")));
-
- EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail1.h"), SM, PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail1.h").get())));
- EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("detail2.h"), SM, PI),
- UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
- Header(FM.getFile("detail2.h").get())));
-
- EXPECT_THAT(findIncludeHeaders(SourceLocFromFile("normal.h"), SM, PI),
- UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
-}
} // namespace
} // namespace clang::include_cleaner
diff --git a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
index 8fdd855c7a1a..e5d8f8839aa1 100644
--- a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_custom_target(ClangIncludeCleanerUnitTests)
add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
AnalysisTest.cpp
+ FindHeadersTest.cpp
RecordTest.cpp
WalkASTTest.cpp
)
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
new file mode 100644
index 000000000000..02854e65fbcb
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -0,0 +1,79 @@
+//===--- FindHeadersTest.cpp ----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Testing/TestAST.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang::include_cleaner {
+namespace {
+using testing::UnorderedElementsAre;
+
+TEST(FindIncludeHeaders, IWYU) {
+ TestInputs Inputs;
+ PragmaIncludes PI;
+ Inputs.MakeAction = [&PI] {
+ struct Hook : public PreprocessOnlyAction {
+ public:
+ Hook(PragmaIncludes *Out) : Out(Out) {}
+ bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+ Out->record(CI);
+ return true;
+ }
+
+ PragmaIncludes *Out;
+ };
+ return std::make_unique<Hook>(&PI);
+ };
+
+ Inputs.Code = R"cpp(
+ #include "header1.h"
+ #include "header2.h"
+ )cpp";
+ Inputs.ExtraFiles["header1.h"] = R"cpp(
+ // IWYU pragma: private, include "path/public.h"
+ )cpp";
+ Inputs.ExtraFiles["header2.h"] = R"cpp(
+ #include "detail1.h" // IWYU pragma: export
+
+ // IWYU pragma: begin_exports
+ #include "detail2.h"
+ // IWYU pragma: end_exports
+
+ #include "normal.h"
+ )cpp";
+ Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
+ Inputs.ExtraFiles["detail2.h"] = "";
+ TestAST AST(Inputs);
+ const auto &SM = AST.sourceManager();
+ auto &FM = SM.getFileManager();
+ // Returns the source location for the start of the file.
+ auto SourceLocFromFile = [&](llvm::StringRef FileName) {
+ return SM.translateFileLineCol(FM.getFile(FileName).get(),
+ /*Line=*/1, /*Col=*/1);
+ };
+
+ EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, PI),
+ UnorderedElementsAre(Header("\"path/public.h\"")));
+
+ EXPECT_THAT(findHeaders(SourceLocFromFile("detail1.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
+ Header(FM.getFile("detail1.h").get())));
+ EXPECT_THAT(findHeaders(SourceLocFromFile("detail2.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
+ Header(FM.getFile("detail2.h").get())));
+
+ EXPECT_THAT(findHeaders(SourceLocFromFile("normal.h"), SM, PI),
+ UnorderedElementsAre(Header(FM.getFile("normal.h").get())));
+}
+
+} // namespace
+} // namespace clang::include_cleaner
\ No newline at end of file
More information about the cfe-commits
mailing list