[clang-tools-extra] 90c5fe9 - [include-cleaner] Allow multiple strategies for spelling includes.
Viktoriia Bakalova via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 5 02:47:27 PDT 2023
Author: Viktoriia Bakalova
Date: 2023-06-05T09:47:12Z
New Revision: 90c5fe9822222190b826aab90c93db9ce0f7e25d
URL: https://github.com/llvm/llvm-project/commit/90c5fe9822222190b826aab90c93db9ce0f7e25d
DIFF: https://github.com/llvm/llvm-project/commit/90c5fe9822222190b826aab90c93db9ce0f7e25d.diff
LOG: [include-cleaner] Allow multiple strategies for spelling includes.
Summary:
Reviewers:
Subscribers:
Differential Revision: https://reviews.llvm.org/D150185
Added:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
Modified:
clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index c7aca83f2ca8c..49e7581d801d9 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyOptions.h"
#include "../utils/OptionsUtils.h"
#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/IncludeSpeller.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
@@ -180,7 +181,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
FileStyle->IncludeStyle);
for (const auto &Inc : Missing) {
std::string Spelling =
- include_cleaner::spellHeader(Inc.Missing, *HS, MainFile);
+ include_cleaner::spellHeader({Inc.Missing, *HS, MainFile});
bool Angled = llvm::StringRef{Spelling}.starts_with("<");
// We might suggest insertion of an existing include in edge cases, e.g.,
// include is present in a PP-disabled region, or spelling of the header
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index d1f6f3a7b06c0..80303267b31a0 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -18,6 +18,7 @@
#include "Selection.h"
#include "SourceCode.h"
#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/IncludeSpeller.h"
#include "clang-include-cleaner/Types.h"
#include "index/SymbolCollector.h"
#include "support/Logger.h"
@@ -1222,7 +1223,9 @@ void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI,
// on local variables, etc.
return;
- HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H);
+ HI.Provider = include_cleaner::spellHeader(
+ {H, AST.getPreprocessor().getHeaderSearchInfo(),
+ SM.getFileEntryForID(SM.getMainFileID())});
}
// FIXME: similar functions are present in FindHeaders.cpp (symbolName)
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index a3e08bc56b31d..c9d32a00fc15f 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -16,6 +16,7 @@
#include "SourceCode.h"
#include "URI.h"
#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/IncludeSpeller.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "support/Logger.h"
@@ -197,8 +198,9 @@ std::vector<Diag> generateMissingIncludeDiagnostics(
continue;
}
- std::string Spelling =
- spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front());
+ std::string Spelling = include_cleaner::spellHeader(
+ {SymbolWithMissingInclude.Providers.front(),
+ AST.getPreprocessor().getHeaderSearchInfo(), MainFile});
llvm::StringRef HeaderRef{Spelling};
bool Angled = HeaderRef.starts_with("<");
// We might suggest insertion of an existing include in edge cases, e.g.,
@@ -332,21 +334,6 @@ convertIncludes(const SourceManager &SM,
return ConvertedIncludes;
}
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
- include_cleaner::Header Provider) {
- if (Provider.kind() == include_cleaner::Header::Physical) {
- if (auto CanonicalPath = getCanonicalPath(Provider.physical()->getLastRef(),
- AST.getSourceManager().getFileManager())) {
- std::string SpelledHeader =
- llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
- if (!SpelledHeader.empty())
- return SpelledHeader;
- }
- }
- return include_cleaner::spellHeader(
- Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
-}
-
std::vector<const Inclusion *>
getUnused(ParsedAST &AST,
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index 675c05a53d5f8..c4051b30ca317 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -74,11 +74,6 @@ include_cleaner::Includes
convertIncludes(const SourceManager &SM,
const llvm::ArrayRef<Inclusion> Includes);
-/// Determines the header spelling of an include-cleaner header
-/// representation. The spelling contains the ""<> characters.
-std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
- include_cleaner::Header Provider);
-
std::vector<include_cleaner::SymbolReference>
collectMacroReferences(ParsedAST &AST);
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 6d764c65a11b2..13a1cfa76c717 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -14,11 +14,12 @@
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/MemoryBufferRef.h"
-#include <variant>
+#include "llvm/ADT/StringRef.h"
+#include <string>
namespace clang {
class SourceLocation;
@@ -75,16 +76,12 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
const format::FormatStyle &IncludeStyle);
-std::string spellHeader(const Header &H, const HeaderSearch &HS,
- const FileEntry *Main);
-
/// Gets all the providers for a symbol by traversing each location.
/// Returned headers are sorted by relevance, first element is the most
/// likely provider for the symbol.
llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
const SourceManager &SM,
const PragmaIncludes *PI);
-
} // namespace include_cleaner
} // namespace clang
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
new file mode 100644
index 0000000000000..98aee5f277cf1
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h
@@ -0,0 +1,49 @@
+//===--- IncludeSpeller.h - Spelling strategies for headers.-------- C++-*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+// An extension point to let applications introduce custom spelling
+// strategies for physical headers.
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_INCLUDE_CLEANER_INCLUDESPELLER_H
+#define CLANG_INCLUDE_CLEANER_INCLUDESPELLER_H
+
+#include "clang-include-cleaner/Types.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "llvm/Support/Registry.h"
+#include <string>
+
+namespace clang::include_cleaner {
+
+/// IncludeSpeller provides an extension point to allow clients implement
+/// custom include spelling strategies for physical headers.
+class IncludeSpeller {
+public:
+ /// Provides the necessary information for custom spelling computations.
+ struct Input {
+ const Header &H;
+ const HeaderSearch &HS;
+ const FileEntry *Main;
+ };
+ virtual ~IncludeSpeller() = default;
+
+ /// Takes in an `Input` struct with necessary infos about a header and
+ /// returns a verbatim include spelling (with angles/quotes) or an empty
+ /// string to indicate no customizations are needed.
+ virtual std::string operator()(const Input &Input) const = 0;
+};
+
+using IncludeSpellingStrategy = llvm::Registry<IncludeSpeller>;
+
+/// Generates a spelling for the header in the `Input` that can be directly
+/// included in the main file. When the `Input` specifies a physical header,
+/// prefers the spelling provided by custom llvm strategies, if any.
+/// Otherwise, uses header search info to generate shortest spelling.
+std::string spellHeader(const IncludeSpeller::Input &Input);
+} // namespace clang::include_cleaner
+
+#endif
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index bf50e064e9811..2c589ae04412a 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -8,10 +8,12 @@
#include "clang-include-cleaner/Analysis.h"
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/IncludeSpeller.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
+#include "clang/Basic/FileEntry.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
#include "clang/Lex/HeaderSearch.h"
@@ -20,10 +22,12 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
#include <string>
namespace clang::include_cleaner {
@@ -53,23 +57,6 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
}
}
-std::string spellHeader(const Header &H, const HeaderSearch &HS,
- const FileEntry *Main) {
- switch (H.kind()) {
- case Header::Physical: {
- bool IsSystem = false;
- std::string Path = HS.suggestPathToFileForDiagnostics(
- H.physical(), Main->tryGetRealPathName(), &IsSystem);
- return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
- }
- case Header::Standard:
- return H.standard().name().str();
- case Header::Verbatim:
- return H.verbatim().str();
- }
- llvm_unreachable("Unknown Header kind");
-}
-
AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
llvm::ArrayRef<SymbolReference> MacroRefs,
const Includes &Inc, const PragmaIncludes *PI,
@@ -90,7 +77,7 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
}
if (!Satisfied && !Providers.empty() &&
Ref.RT == RefType::Explicit)
- Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+ Missing.insert(spellHeader({Providers.front(), HS, MainFile}));
});
AnalysisResults Results;
diff --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index 75e1fb725c656..7b6d4991eeff3 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS Support)
add_clang_library(clangIncludeCleaner
Analysis.cpp
+ IncludeSpeller.cpp
FindHeaders.cpp
HTMLReport.cpp
LocateSymbol.cpp
diff --git a/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp b/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
new file mode 100644
index 0000000000000..e29a8104c0ae8
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
@@ -0,0 +1,66 @@
+//===--- IncludeSpeller.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 "clang-include-cleaner/IncludeSpeller.h"
+#include "clang-include-cleaner/Types.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Registry.h"
+#include <memory>
+#include <string>
+
+LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy)
+
+namespace clang::include_cleaner {
+namespace {
+
+// Fallback strategy to default spelling via header search.
+class DefaultIncludeSpeller : public IncludeSpeller {
+public:
+ std::string operator()(const Input &Input) const override {
+ bool IsSystem = false;
+ std::string FinalSpelling = Input.HS.suggestPathToFileForDiagnostics(
+ Input.H.physical(), Input.Main->tryGetRealPathName(), &IsSystem);
+ return IsSystem ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\"";
+ }
+};
+
+std::string spellPhysicalHeader(const IncludeSpeller::Input &Input) {
+ static auto Spellers = [] {
+ llvm::SmallVector<std::unique_ptr<include_cleaner::IncludeSpeller>> Result;
+ for (const auto &Strategy :
+ include_cleaner::IncludeSpellingStrategy::entries())
+ Result.push_back(Strategy.instantiate());
+ Result.push_back(std::make_unique<DefaultIncludeSpeller>());
+ return Result;
+ }();
+
+ std::string Spelling;
+ for (const auto &Speller : Spellers) {
+ Spelling = (*Speller)(Input);
+ if (!Spelling.empty())
+ break;
+ }
+ return Spelling;
+}
+} // namespace
+
+std::string spellHeader(const IncludeSpeller::Input &Input) {
+ const Header &H = Input.H;
+ switch (H.kind()) {
+ case Header::Standard:
+ return H.standard().name().str();
+ case Header::Verbatim:
+ return H.verbatim().str();
+ case Header::Physical:
+ // Spelling physical headers allows for various plug-in strategies.
+ return spellPhysicalHeader(Input);
+ }
+ llvm_unreachable("Unknown Header kind");
+}
+} // namespace clang::include_cleaner
\ No newline at end of file
diff --git a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
index bd9729f48a149..89b55f4c7b190 100644
--- a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
@@ -7,6 +7,7 @@ add_custom_target(ClangIncludeCleanerUnitTests)
add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
AnalysisTest.cpp
FindHeadersTest.cpp
+ IncludeSpellerTest.cpp
LocateSymbolTest.cpp
RecordTest.cpp
TypesTest.cpp
diff --git a/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp b/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
new file mode 100644
index 0000000000000..1dd4d28ed81be
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
@@ -0,0 +1,78 @@
+//===--- IncludeSpellerTest.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 "clang-include-cleaner/IncludeSpeller.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Testing/TestAST.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+#include <assert.h>
+#include <string>
+namespace clang::include_cleaner {
+namespace {
+
+const char *testRoot() {
+#ifdef _WIN32
+ return "C:\\include-cleaner-test";
+#else
+ return "/include-cleaner-test";
+#endif
+}
+
+std::string testPath(llvm::StringRef File) {
+ assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
+
+ llvm::SmallString<32> NativeFile = File;
+ llvm::sys::path::native(NativeFile, llvm::sys::path::Style::native);
+ llvm::SmallString<32> Path;
+ llvm::sys::path::append(Path, llvm::sys::path::Style::native, testRoot(),
+ NativeFile);
+ return std::string(Path.str());
+}
+
+class DummyIncludeSpeller : public IncludeSpeller {
+public:
+ std::string operator()(const IncludeSpeller::Input &Input) const override {
+ llvm::StringRef AbsolutePath = Input.H.physical()->tryGetRealPathName();
+ std::string RootWithSeparator{testRoot()};
+ RootWithSeparator += llvm::sys::path::get_separator();
+ if (!AbsolutePath.consume_front(llvm::StringRef{RootWithSeparator}))
+ return "";
+ return "\"" + AbsolutePath.str() + "\"";
+ }
+};
+
+TEST(IncludeSpeller, IsRelativeToTestRoot) {
+ TestInputs Inputs;
+
+ Inputs.ExtraArgs.push_back("-isystemdir");
+
+ Inputs.ExtraFiles[testPath("foo.h")] = "";
+ Inputs.ExtraFiles["dir/header.h"] = "";
+ TestAST AST{Inputs};
+
+ auto &FM = AST.fileManager();
+ auto &HS = AST.preprocessor().getHeaderSearchInfo();
+ const auto *MainFile = AST.sourceManager().getFileEntryForID(
+ AST.sourceManager().getMainFileID());
+
+ EXPECT_EQ("\"foo.h\"", spellHeader({Header{*FM.getFile(testPath("foo.h"))},
+ HS, MainFile}));
+ EXPECT_EQ("<header.h>",
+ spellHeader({Header{*FM.getFile("dir/header.h")}, HS, MainFile}));
+}
+
+IncludeSpellingStrategy::Add<DummyIncludeSpeller>
+ Speller("dummy", "Dummy Include Speller");
+
+} // namespace
+} // namespace clang::include_cleaner
More information about the cfe-commits
mailing list