VitaNuo updated this revision to Diff 525581.
VitaNuo added a comment.

Move the speller out of the loop.


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/lib/Analysis.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
@@ -12,6 +12,7 @@
 #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"
@@ -24,8 +25,12 @@
 #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 <string>
 
+LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy)
+
 namespace clang::include_cleaner {
 
 void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
@@ -54,20 +59,32 @@
 }
 
 std::string spellHeader(const Header &H, 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:
+                        const FileEntry *Main,
+                        const IncludeSpeller *CustomSpeller) {
+  if (H.kind() == Header::Standard) {
     return H.standard().name().str();
-  case Header::Verbatim:
+  }
+
+  if (H.kind() == Header::Verbatim) {
     return H.verbatim().str();
   }
-  llvm_unreachable("Unknown Header kind");
+
+  if (H.kind() != Header::Physical)
+    llvm_unreachable("Unknown Header kind");
+
+  // Spelling physical headers allows for various plug-in strategies.
+  std::string FinalSpelling;
+  if (CustomSpeller)
+    FinalSpelling = (*CustomSpeller)(H.physical()->tryGetRealPathName());
+
+  if (!FinalSpelling.empty())
+    return FinalSpelling;
+
+  // Fallback to default spelling via header search.
+  bool IsSystem = false;
+  std::string Path = HS.suggestPathToFileForDiagnostics(
+      H.physical(), Main->tryGetRealPathName(), &IsSystem);
+  return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
 }
 
 AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
@@ -89,8 +106,11 @@
                }
              }
              if (!Satisfied && !Providers.empty() &&
-                 Ref.RT == RefType::Explicit)
-               Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+                 Ref.RT == RefType::Explicit) {
+               ApplyFirstIncludeSpeller Speller;
+               Missing.insert(
+                   spellHeader(Providers.front(), HS, MainFile, &Speller));
+             }
            });
 
   AnalysisResults Results;
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
@@ -14,11 +14,14 @@
 #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 <memory>
+#include <string>
 
 namespace clang {
 class SourceLocation;
@@ -75,9 +78,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 +85,39 @@
                                            const SourceManager &SM,
                                            const PragmaIncludes *PI);
 
+class IncludeSpeller {
+public:
+  virtual ~IncludeSpeller() = default;
+
+  virtual std::string operator()(llvm::StringRef HeaderPhysicalPath) const = 0;
+};
+
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+                        const FileEntry *Main,
+                        const IncludeSpeller *CustomSpeller);
+
+typedef llvm::Registry<IncludeSpeller> IncludeSpellingStrategy;
+
+class ApplyFirstIncludeSpeller : public IncludeSpeller {
+public:
+  ApplyFirstIncludeSpeller() {
+    for (const auto &Strategy :
+         include_cleaner::IncludeSpellingStrategy::entries()) {
+      Spellers.push_back(Strategy.instantiate());
+    }
+  }
+  std::string operator()(llvm::StringRef HeaderPhysicalPath) const override {
+    for (const auto &Spell : Spellers) {
+      std::string Spelling = (*Spell)(HeaderPhysicalPath);
+      if (!Spelling.empty())
+        return Spelling;
+    }
+    return "";
+  }
+
+private:
+  llvm::SmallVector<std::unique_ptr<include_cleaner::IncludeSpeller>> Spellers;
+};
 } // 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());
@@ -187,6 +186,7 @@
 
   tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code,
                                          FileStyle->IncludeStyle);
+  include_cleaner::ApplyFirstIncludeSpeller Speller;
   for (const auto &SymbolWithMissingInclude : MissingIncludes) {
     llvm::StringRef ResolvedPath =
         getResolvedPath(SymbolWithMissingInclude.Providers.front());
@@ -197,8 +197,9 @@
       continue;
     }
 
-    std::string Spelling =
-        spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front());
+    std::string Spelling = include_cleaner::spellHeader(
+        SymbolWithMissingInclude.Providers.front(),
+        AST.getPreprocessor().getHeaderSearchInfo(), MainFile, &Speller);
     llvm::StringRef HeaderRef{Spelling};
     bool Angled = HeaderRef.starts_with("<");
     // We might suggest insertion of an existing include in edge cases, e.g.,
@@ -332,21 +333,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,
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1222,7 +1222,10 @@
     // on local variables, etc.
     return;
 
-  HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H);
+  include_cleaner::ApplyFirstIncludeSpeller Speller;
+  HI.Provider = include_cleaner::spellHeader(H,
+        AST.getPreprocessor().getHeaderSearchInfo(),
+        SM.getFileEntryForID(SM.getMainFileID()), &Speller);
 }
 
 // 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

Reply via email to