[clang-tools-extra] ed365f4 - [clangd] Use FileEntryRef for canonicalizing filepaths.

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 13 07:49:57 PDT 2023


Author: Utkarsh Saxena
Date: 2023-04-13T16:49:30+02:00
New Revision: ed365f464a0a29da08d0a1011603c4cd337c9428

URL: https://github.com/llvm/llvm-project/commit/ed365f464a0a29da08d0a1011603c4cd337c9428
DIFF: https://github.com/llvm/llvm-project/commit/ed365f464a0a29da08d0a1011603c4cd337c9428.diff

LOG: [clangd] Use FileEntryRef for canonicalizing filepaths.

Using FileEntry for retrieving filenames give the name used for the last access. FileEntryRef gives the first access name.

Last access name is suspected to change with unrelated changes in clang or the underlying filesystem. First access name gives more stability to the name and makes it easier to track.

Differential Revision: https://reviews.llvm.org/D148213

Added: 
    

Modified: 
    clang-tools-extra/clangd/Diagnostics.cpp
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/SourceCode.cpp
    clang-tools-extra/clangd/SourceCode.h
    clang-tools-extra/clangd/XRefs.cpp
    clang-tools-extra/clangd/index/Background.cpp
    clang-tools-extra/clangd/index/SymbolCollector.cpp
    clang-tools-extra/clangd/indexer/IndexerMain.cpp
    clang-tools-extra/clangd/refactor/Tweak.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index 12a865fcf9e50..963bf02e21848 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -715,9 +715,9 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
     D.InsideMainFile = isInsideMainFile(PatchLoc, SM);
     D.Range = diagnosticRange(Info, *LangOpts);
     auto FID = SM.getFileID(Info.getLocation());
-    if (auto *FE = SM.getFileEntryForID(FID)) {
+    if (const auto FE = SM.getFileEntryRefForID(FID)) {
       D.File = FE->getName().str();
-      D.AbsFile = getCanonicalPath(FE, SM);
+      D.AbsFile = getCanonicalPath(*FE, SM);
     }
     D.ID = Info.getID();
     return D;

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index b0e4d8dee5568..168471a603ea4 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -317,8 +317,8 @@ convertIncludes(const SourceManager &SM,
 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(), AST.getSourceManager())) {
+    if (auto CanonicalPath = getCanonicalPath(Provider.physical()->getLastRef(),
+                                              AST.getSourceManager())) {
       std::string SpelledHeader =
           llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
       if (!SpelledHeader.empty())

diff  --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 044562ec1b59a..831adc3d5fd8d 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -12,6 +12,7 @@
 #include "Protocol.h"
 #include "support/Context.h"
 #include "support/Logger.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -512,12 +513,9 @@ std::vector<TextEdit> replacementsToEdits(llvm::StringRef Code,
   return Edits;
 }
 
-std::optional<std::string> getCanonicalPath(const FileEntry *F,
+std::optional<std::string> getCanonicalPath(const FileEntryRef F,
                                             const SourceManager &SourceMgr) {
-  if (!F)
-    return std::nullopt;
-
-  llvm::SmallString<128> FilePath = F->getName();
+  llvm::SmallString<128> FilePath = F.getName();
   if (!llvm::sys::path::is_absolute(FilePath)) {
     if (auto EC =
             SourceMgr.getFileManager().getVirtualFileSystem().makeAbsolute(

diff  --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index 8643fab09656f..8b7c028eb2478 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -163,7 +163,7 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
 /// This function should be used when paths needs to be used outside the
 /// component that generate it, so that paths are normalized as much as
 /// possible.
-std::optional<std::string> getCanonicalPath(const FileEntry *F,
+std::optional<std::string> getCanonicalPath(const FileEntryRef F,
                                             const SourceManager &SourceMgr);
 
 /// Choose the clang-format style we should apply to a certain file.

diff  --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index d4442c11a8ed0..9a1bd7dbd2564 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -208,10 +208,10 @@ getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations,
 std::optional<Location> makeLocation(const ASTContext &AST, SourceLocation Loc,
                                      llvm::StringRef TUPath) {
   const auto &SM = AST.getSourceManager();
-  const FileEntry *F = SM.getFileEntryForID(SM.getFileID(Loc));
+  const auto F = SM.getFileEntryRefForID(SM.getFileID(Loc));
   if (!F)
     return std::nullopt;
-  auto FilePath = getCanonicalPath(F, SM);
+  auto FilePath = getCanonicalPath(*F, SM);
   if (!FilePath) {
     log("failed to get path!");
     return std::nullopt;
@@ -1617,8 +1617,10 @@ declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
       toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc});
   if (!DeclRange)
     return std::nullopt;
-  auto FilePath =
-      getCanonicalPath(SM.getFileEntryForID(SM.getFileID(NameLoc)), SM);
+  const auto FE = SM.getFileEntryRefForID(SM.getFileID(NameLoc));
+  if (!FE)
+    return std::nullopt;
+  auto FilePath = getCanonicalPath(*FE, SM);
   if (!FilePath)
     return std::nullopt; // Not useful without a uri.
 
@@ -1968,7 +1970,8 @@ static void unwrapFindType(
     return unwrapFindType(FT->getReturnType(), H, Out);
   if (auto *CRD = T->getAsCXXRecordDecl()) {
     if (CRD->isLambda())
-      return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType(), H, Out);
+      return unwrapFindType(CRD->getLambdaCallOperator()->getReturnType(), H,
+                            Out);
     // FIXME: more cases we'd prefer the return type of the call operator?
     //        std::function etc?
   }

diff  --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index 117c766eea0dd..73330b3ae6a86 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -288,10 +288,10 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
   // digests.
   IndexOpts.FileFilter = [&ShardVersionsSnapshot](const SourceManager &SM,
                                                   FileID FID) {
-    const auto *F = SM.getFileEntryForID(FID);
+    const auto F = SM.getFileEntryRefForID(FID);
     if (!F)
       return false; // Skip invalid files.
-    auto AbsPath = getCanonicalPath(F, SM);
+    auto AbsPath = getCanonicalPath(*F, SM);
     if (!AbsPath)
       return false; // Skip files without absolute path.
     auto Digest = digestFile(SM, FID);

diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 3179810b1b185..5c8112e60b224 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -205,11 +205,11 @@ class SymbolCollector::HeaderFileURICache {
 
   // Returns a canonical URI for the file \p FE.
   // We attempt to make the path absolute first.
-  const std::string &toURI(const FileEntry *FE) {
+  const std::string &toURI(const FileEntryRef FE) {
     auto R = CacheFEToURI.try_emplace(FE);
     if (R.second) {
       auto CanonPath = getCanonicalPath(FE, SM);
-      R.first->second = &toURIInternal(CanonPath ? *CanonPath : FE->getName());
+      R.first->second = &toURIInternal(CanonPath ? *CanonPath : FE.getName());
     }
     return *R.first->second;
   }
@@ -218,7 +218,7 @@ class SymbolCollector::HeaderFileURICache {
   // If the file is in the FileManager, use that to canonicalize the path.
   // We attempt to make the path absolute in any case.
   const std::string &toURI(llvm::StringRef Path) {
-    if (auto File = SM.getFileManager().getFile(Path))
+    if (auto File = SM.getFileManager().getFileRef(Path))
       return toURI(*File);
     return toURIInternal(Path);
   }
@@ -373,7 +373,7 @@ class SymbolCollector::HeaderFileURICache {
   }
 
   llvm::StringRef getIncludeHeaderUncached(FileID FID) {
-    const FileEntry *FE = SM.getFileEntryForID(FID);
+    const auto FE = SM.getFileEntryRefForID(FID);
     if (!FE || FE->getName().empty())
       return "";
     llvm::StringRef Filename = FE->getName();
@@ -392,13 +392,13 @@ class SymbolCollector::HeaderFileURICache {
     // Framework headers are spelled as <FrameworkName/Foo.h>, not
     // "path/FrameworkName.framework/Headers/Foo.h".
     auto &HS = PP->getHeaderSearchInfo();
-    if (const auto *HFI = HS.getExistingFileInfo(FE, /*WantExternal*/ false))
+    if (const auto *HFI = HS.getExistingFileInfo(*FE, /*WantExternal*/ false))
       if (!HFI->Framework.empty())
         if (auto Spelling =
-                getFrameworkHeaderIncludeSpelling(FE, HFI->Framework, HS))
+                getFrameworkHeaderIncludeSpelling(*FE, HFI->Framework, HS))
           return *Spelling;
 
-    if (!tooling::isSelfContainedHeader(FE, PP->getSourceManager(),
+    if (!tooling::isSelfContainedHeader(*FE, PP->getSourceManager(),
                                         PP->getHeaderSearchInfo())) {
       // A .inc or .def file is often included into a real header to define
       // symbols (e.g. LLVM tablegen files).
@@ -409,7 +409,7 @@ class SymbolCollector::HeaderFileURICache {
       return "";
     }
     // Standard case: just insert the file itself.
-    return toURI(FE);
+    return toURI(*FE);
   }
 };
 
@@ -417,12 +417,12 @@ class SymbolCollector::HeaderFileURICache {
 std::optional<SymbolLocation>
 SymbolCollector::getTokenLocation(SourceLocation TokLoc) {
   const auto &SM = ASTCtx->getSourceManager();
-  auto *FE = SM.getFileEntryForID(SM.getFileID(TokLoc));
+  const auto FE = SM.getFileEntryRefForID(SM.getFileID(TokLoc));
   if (!FE)
     return std::nullopt;
 
   SymbolLocation Result;
-  Result.FileURI = HeaderFileURIs->toURI(FE).c_str();
+  Result.FileURI = HeaderFileURIs->toURI(*FE).c_str();
   auto Range = getTokenRange(TokLoc, SM, ASTCtx->getLangOpts());
   Result.Start = Range.first;
   Result.End = Range.second;
@@ -635,10 +635,10 @@ bool SymbolCollector::handleDeclOccurrence(
 void SymbolCollector::handleMacros(const MainFileMacros &MacroRefsToIndex) {
   assert(HeaderFileURIs && PP);
   const auto &SM = PP->getSourceManager();
-  const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
-  assert(MainFileEntry);
+  const auto MainFileEntryRef = SM.getFileEntryRefForID(SM.getMainFileID());
+  assert(MainFileEntryRef);
 
-  const std::string &MainFileURI = HeaderFileURIs->toURI(MainFileEntry);
+  const std::string &MainFileURI = HeaderFileURIs->toURI(*MainFileEntryRef);
   // Add macro references.
   for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
     for (const auto &MacroRef : IDToRefs.second) {
@@ -987,12 +987,12 @@ void SymbolCollector::addRef(SymbolID ID, const SymbolRef &SR) {
   const auto &SM = ASTCtx->getSourceManager();
   // FIXME: use the result to filter out references.
   shouldIndexFile(SR.FID);
-  if (const auto *FE = SM.getFileEntryForID(SR.FID)) {
+  if (const auto FE = SM.getFileEntryRefForID(SR.FID)) {
     auto Range = getTokenRange(SR.Loc, SM, ASTCtx->getLangOpts());
     Ref R;
     R.Location.Start = Range.first;
     R.Location.End = Range.second;
-    R.Location.FileURI = HeaderFileURIs->toURI(FE).c_str();
+    R.Location.FileURI = HeaderFileURIs->toURI(*FE).c_str();
     R.Kind = toRefKind(SR.Roles, SR.Spelled);
     R.Container = getSymbolIDCached(SR.Container);
     Refs.insert(ID, R);

diff  --git a/clang-tools-extra/clangd/indexer/IndexerMain.cpp b/clang-tools-extra/clangd/indexer/IndexerMain.cpp
index 9070582801f21..a66ab91c04231 100644
--- a/clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ b/clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -46,10 +46,10 @@ class IndexActionFactory : public tooling::FrontendActionFactory {
     SymbolCollector::Options Opts;
     Opts.CountReferences = true;
     Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
-      const auto *F = SM.getFileEntryForID(FID);
+      const auto F = SM.getFileEntryRefForID(FID);
       if (!F)
         return false; // Skip invalid files.
-      auto AbsPath = getCanonicalPath(F, SM);
+      auto AbsPath = getCanonicalPath(*F, SM);
       if (!AbsPath)
         return false; // Skip files without absolute path.
       std::lock_guard<std::mutex> Lock(FilesMu);

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp
index bf11c2c5ac5da..bfa8bf6984f4b 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -104,8 +104,9 @@ llvm::Expected<std::pair<Path, Edit>>
 Tweak::Effect::fileEdit(const SourceManager &SM, FileID FID,
                         tooling::Replacements Replacements) {
   Edit Ed(SM.getBufferData(FID), std::move(Replacements));
-  if (auto FilePath = getCanonicalPath(SM.getFileEntryForID(FID), SM))
-    return std::make_pair(*FilePath, std::move(Ed));
+  if (const auto FE = SM.getFileEntryRefForID(FID))
+    if (auto FilePath = getCanonicalPath(*FE, SM))
+      return std::make_pair(*FilePath, std::move(Ed));
   return error("Failed to get absolute path for edited file: {0}",
                SM.getFileEntryRefForID(FID)->getName());
 }


        


More information about the cfe-commits mailing list