VitaNuo updated this revision to Diff 527435. VitaNuo added a comment. Update.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: 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/include/clang-include-cleaner/IncludeSpeller.h clang-tools-extra/include-cleaner/lib/Analysis.cpp clang-tools-extra/include-cleaner/unittests/CMakeLists.txt clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp =================================================================== --- /dev/null +++ clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp @@ -0,0 +1,86 @@ +//===--- 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 <initializer_list> +#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 IncludeSpellerInput &Input) const override { + llvm::StringRef AbsolutePath = Input.H.physical()->tryGetRealPathName(); + std::string RootWithSeparator{testRoot()}; + RootWithSeparator += llvm::sys::path::get_separator(); + bool Result = + AbsolutePath.consume_front(llvm::StringRef{RootWithSeparator}); + if (!Result) + return ""; + return AbsolutePath.str(); + } +}; + +TEST(IncludeSpeller, IsRelativeToTestRoot) { + TestInputs Inputs; + Inputs.FileName = testPath("foo.h"); + Inputs.ExtraFiles["bar.h"] = ""; + Inputs.ExtraFiles[testPath("foo/baz.h")] = ""; + Inputs.ExtraFiles["/foo/bar.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("\"bar.h\"", + spellHeader({Header{*FM.getFile("bar.h")}, HS, MainFile})); + EXPECT_EQ( + "foo/baz.h", + spellHeader({Header{*FM.getFile(testPath("foo/baz.h"))}, HS, MainFile})); + EXPECT_EQ("\"/foo/bar.h\"", + spellHeader({Header{*FM.getFile("/foo/bar.h")}, HS, MainFile})); +} + +IncludeSpellingStrategy::Add<DummyIncludeSpeller> + Speller("dummy", "Dummy Include Speller"); + +} // namespace +} // namespace clang::include_cleaner \ No newline at end of file Index: clang-tools-extra/include-cleaner/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/include-cleaner/unittests/CMakeLists.txt +++ clang-tools-extra/include-cleaner/unittests/CMakeLists.txt @@ -7,6 +7,7 @@ add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests AnalysisTest.cpp FindHeadersTest.cpp + IncludeSpellerTest.cpp LocateSymbolTest.cpp RecordTest.cpp TypesTest.cpp Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp =================================================================== --- clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ 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,14 +22,39 @@ #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 "llvm/Support/Registry.h" +#include <memory> #include <string> +LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy) + namespace clang::include_cleaner { +std::string mapHeader(const IncludeSpellerInput &Input) { + static auto Spellers = [] { + auto Result = + llvm::SmallVector<std::unique_ptr<include_cleaner::IncludeSpeller>>{}; + for (const auto &Strategy : + include_cleaner::IncludeSpellingStrategy::entries()) + Result.push_back(Strategy.instantiate()); + return Result; + }(); + + std::string Spelling; + for (const auto &Speller : Spellers) { + Spelling = (*Speller)(Input); + if (!Spelling.empty()) + break; + } + return Spelling; +} + void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, llvm::ArrayRef<SymbolReference> MacroRefs, const PragmaIncludes *PI, const SourceManager &SM, @@ -53,19 +80,24 @@ } } -std::string spellHeader(const Header &H, HeaderSearch &HS, - const FileEntry *Main) { +std::string spellHeader(const IncludeSpellerInput &Input) { + const Header &H = Input.H; 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(); + case Header::Physical: + // Spelling physical headers allows for various plug-in strategies. + std::string FinalSpelling = mapHeader(Input); + if (!FinalSpelling.empty()) + return FinalSpelling; + + // Fallback to default spelling via header search. + bool IsSystem = false; + FinalSpelling = Input.HS.suggestPathToFileForDiagnostics( + H.physical(), Input.Main->tryGetRealPathName(), &IsSystem); + return IsSystem ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\""; } llvm_unreachable("Unknown Header kind"); } @@ -90,7 +122,7 @@ } if (!Satisfied && !Providers.empty() && Ref.RT == RefType::Explicit) - Missing.insert(spellHeader(Providers.front(), HS, MainFile)); + Missing.insert(spellHeader({Providers.front(), HS, MainFile})); }); AnalysisResults Results; Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h =================================================================== --- /dev/null +++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h @@ -0,0 +1,39 @@ +//===--- 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 <string> + +namespace clang::include_cleaner { + +/// Provides the necessary information for custom spelling computations. +struct IncludeSpellerInput { + const Header &H; + HeaderSearch &HS; + const FileEntry *Main; +}; + +class IncludeSpeller { +public: + virtual ~IncludeSpeller() = default; + + /// Takes in absolute path to a source file and should return a verbatim + /// include spelling (with angles/quotes) or an empty string to indicate no + /// customizations are needed. + virtual std::string operator()(const IncludeSpellerInput &Input) const = 0; +}; +} // namespace clang::include_cleaner + +#endif \ No newline at end of file Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h =================================================================== --- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h @@ -11,14 +11,18 @@ #ifndef CLANG_INCLUDE_CLEANER_ANALYSIS_H #define 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/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 "llvm/Support/Registry.h" +#include <functional> +#include <string> namespace clang { class SourceLocation; @@ -75,9 +79,6 @@ std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code, const format::FormatStyle &IncludeStyle); -std::string spellHeader(const Header &H, 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. @@ -85,6 +86,18 @@ const SourceManager &SM, const PragmaIncludes *PI); +using IncludeSpellingStrategy = llvm::Registry<IncludeSpeller>; + +/// A header mapper that iterates over all registered include spelling +/// strategies. Note that when there are multiple strategies iteration +/// order is not specified. +std::function<std::string(llvm::StringRef)> defaultHeaderMapper(); + +/// Generates a spelling for `H` that can be directly included in `Main`. +/// When `H` is 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 IncludeSpellerInput &Input); } // namespace include_cleaner } // namespace clang Index: clang-tools-extra/clangd/IncludeCleaner.h =================================================================== --- clang-tools-extra/clangd/IncludeCleaner.h +++ clang-tools-extra/clangd/IncludeCleaner.h @@ -74,11 +74,6 @@ 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); Index: clang-tools-extra/clangd/IncludeCleaner.cpp =================================================================== --- clang-tools-extra/clangd/IncludeCleaner.cpp +++ clang-tools-extra/clangd/IncludeCleaner.cpp @@ -176,7 +176,6 @@ const SourceManager &SM = AST.getSourceManager(); const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); - auto FileStyle = format::getStyle( format::DefaultFormatStyle, AST.tuPath(), format::DefaultFallbackStyle, Code, &SM.getFileManager().getVirtualFileSystem()); @@ -197,8 +196,9 @@ 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 +332,6 @@ 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())) { - 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, @@ -456,7 +441,8 @@ return {std::move(UnusedIncludes), std::move(MissingIncludes)}; } -std::optional<Fix> removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) { +std::optional<Fix> +removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) { if (UnusedIncludes.empty()) return std::nullopt; @@ -465,8 +451,8 @@ for (const auto &Diag : UnusedIncludes) { assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); RemoveAll.Edits.insert(RemoveAll.Edits.end(), - Diag.Fixes.front().Edits.begin(), - Diag.Fixes.front().Edits.end()); + Diag.Fixes.front().Edits.begin(), + Diag.Fixes.front().Edits.end()); } // TODO(hokein): emit a suitable text for the label. @@ -494,7 +480,7 @@ llvm::StringMap<TextEdit> Edits; for (const auto &Diag : MissingIncludeDiags) { assert(Diag.Fixes.size() == 1 && "Expected exactly one fix."); - for (const auto& Edit : Diag.Fixes.front().Edits) { + for (const auto &Edit : Diag.Fixes.front().Edits) { Edits.try_emplace(Edit.newText, Edit); } } @@ -514,7 +500,7 @@ } return AddAllMissing; } -Fix fixAll(const Fix& RemoveAllUnused, const Fix& AddAllMissing) { +Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) { Fix FixAll; FixAll.Message = "fix all includes"; @@ -523,22 +509,23 @@ for (const auto &F : AddAllMissing.Edits) FixAll.Edits.push_back(F); - for (const auto& A : RemoveAllUnused.Annotations) + for (const auto &A : RemoveAllUnused.Annotations) FixAll.Annotations.push_back(A); - for (const auto& A : AddAllMissing.Annotations) + for (const auto &A : AddAllMissing.Annotations) FixAll.Annotations.push_back(A); return FixAll; } -std::vector<Diag> generateIncludeCleanerDiagnostic( - ParsedAST &AST, const IncludeCleanerFindings &Findings, - llvm::StringRef Code) { +std::vector<Diag> +generateIncludeCleanerDiagnostic(ParsedAST &AST, + const IncludeCleanerFindings &Findings, + llvm::StringRef Code) { std::vector<Diag> UnusedIncludes = generateUnusedIncludeDiagnostics( AST.tuPath(), Findings.UnusedIncludes, Code); std::optional<Fix> RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes); - std::vector<Diag> MissingIncludeDiags = generateMissingIncludeDiagnostics( - AST, Findings.MissingIncludes, Code); + std::vector<Diag> MissingIncludeDiags = + generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code); std::optional<Fix> AddAllMissing = addAllMissingIncludes(MissingIncludeDiags); std::optional<Fix> FixAll; @@ -546,26 +533,23 @@ FixAll = fixAll(*RemoveAllUnused, *AddAllMissing); auto AddBatchFix = [](const std::optional<Fix> &F, clang::clangd::Diag *Out) { - if (!F) return; + if (!F) + return; Out->Fixes.push_back(*F); }; for (auto &Diag : MissingIncludeDiags) { - AddBatchFix(MissingIncludeDiags.size() > 1 - ? AddAllMissing - : std::nullopt, + AddBatchFix(MissingIncludeDiags.size() > 1 ? AddAllMissing : std::nullopt, &Diag); AddBatchFix(FixAll, &Diag); } for (auto &Diag : UnusedIncludes) { - AddBatchFix(UnusedIncludes.size() > 1 ? RemoveAllUnused - : std::nullopt, + AddBatchFix(UnusedIncludes.size() > 1 ? RemoveAllUnused : std::nullopt, &Diag); AddBatchFix(FixAll, &Diag); } auto Result = std::move(MissingIncludeDiags); - llvm::move(UnusedIncludes, - std::back_inserter(Result)); + llvm::move(UnusedIncludes, std::back_inserter(Result)); return Result; } Index: clang-tools-extra/clangd/Hover.cpp =================================================================== --- clang-tools-extra/clangd/Hover.cpp +++ clang-tools-extra/clangd/Hover.cpp @@ -1222,7 +1222,9 @@ // 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)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
