[clangd] Use FileEntryRef for canonicalizing filepaths.

[usaxena95]

Summary

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.

[usaxena95]

More refactorings.

[usaxena95]

Rebase

[kadircet]

we've got one more reference to getCanonicalPath in clang-tools-extra/clangd/IncludeCleaner.cpp:320, i guess the best way is just calling getLastRef on the FileEntry*

clang-tools-extra/clangd/SourceCode.cpp

517–518

we don't need this check anymore

clang-tools-extra/clangd/SourceCode.h

166

just FileEntryRef, it's a cheap value type that provides an immutable view into underlying FileEntry

clang-tools-extra/clangd/index/SymbolCollector.cpp

208

again just FileEntryRef

395

FileEntryRef should be implicitly convertable to FileEntry* (so just *FE should be enough?)

399

same as above

401

same as above

[usaxena95]

Addressed comments.

[kadircet]

thanks!

clang-tools-extra/clangd/Diagnostics.cpp

void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
   auto FillDiagBase = [&](DiagBase &D) {
     fillNonLocationData(DiagLevel, Info, D);
 
     SourceLocation PatchLoc =
         translatePreamblePatchLocation(Info.getLocation(), SM);
     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;
   };
 
   auto AddFix = [&](bool SyntheticMessage) -> bool {
     assert(!Info.getFixItHints().empty() &&
            "diagnostic does not have attached fix-its");

clang-tools-extra/clangd/IncludeCleaner.cpp

for (const Inclusion &Inc : Includes) {
     ConvertedIncludes.add(std::move(TransformedInc));
   }
   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(), AST.getSourceManager())) {
+    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);

clang-tools-extra/clangd/SourceCode.h

 ///   - Absolute path
 ///   - Symlinks resolved
 ///   - No "." or ".." component
 ///   - No duplicate or trailing directory separator
 ///
 /// 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.
 /// This will usually use FS to look for .clang-format directories.
 /// FIXME: should we be caching the .clang-format file search?
 /// This uses format::DefaultFormatStyle and format::DefaultFallbackStyle,
 /// though the latter may have been overridden in main()!
 format::FormatStyle getFormatStyleForFile(llvm::StringRef File,

clang-tools-extra/clangd/SourceCode.cpp

 //===--- SourceCode.h - Manipulating source code as strings -----*- 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
 //
 //===----------------------------------------------------------------------===//
 #include "SourceCode.h"
 
 #include "FuzzyMatch.h"
 #include "Preamble.h"
 #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"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Driver/Types.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"

 std::vector<TextEdit> replacementsToEdits(llvm::StringRef Code,
                                           const tooling::Replacements &Repls) {
   std::vector<TextEdit> Edits;
   for (const auto &R : Repls)
     Edits.push_back(replacementToEdit(Code, R));
   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(
                 FilePath)) {
       elog("Could not turn relative path '{0}' to absolute: {1}", FilePath,
            EC.message());
       return std::nullopt;
     }

clang-tools-extra/clangd/XRefs.cpp

getDeclAtPosition(ParsedAST &AST, SourceLocation Pos, DeclRelationSet Relations,
   return Result;
 }
 
 // Expects Loc to be a SpellingLocation, will bail out otherwise as it can't
 // figure out a filename.
 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;
   }
   Location L;
   L.uri = URIForFile::canonicalize(*FilePath, TUPath);
   // We call MeasureTokenLength here as TokenBuffer doesn't store spelled tokens
   // outside the main file.
declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
   auto &SM = Ctx.getSourceManager();
   SourceLocation NameLoc = nameLocation(ND, Ctx.getSourceManager());
   SourceLocation BeginLoc = SM.getFileLoc(ND.getBeginLoc());
   SourceLocation EndLoc = SM.getFileLoc(ND.getEndLoc());
   const auto DeclRange =
       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.
 
   Position NameBegin = sourceLocToPosition(SM, NameLoc);
   Position NameEnd = sourceLocToPosition(
       SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
static void unwrapFindType(
   if (const auto *AT = T->getAsArrayTypeUnsafe())
     return unwrapFindType(AT->getElementType(), H, Out);
 
   // Function type => return type.
   if (auto *FT = T->getAs<FunctionType>())
     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?
   }
 
   // For smart pointer types, add the underlying type
   if (H)
     if (const auto* PointeeType = H->getPointeeType(T.getNonReferenceType().getTypePtr())) {
         unwrapFindType(QualType(PointeeType, 0), H, Out);

clang-tools-extra/clangd/index/Background.cpp

llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
   if (!Clang)
     return error("Couldn't build compiler instance");
 
   SymbolCollector::Options IndexOpts;
   // Creates a filter to not collect index results from files with unchanged
   // 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);
     if (!Digest)
       return false;
     auto D = ShardVersionsSnapshot.find(*AbsPath);
     if (D != ShardVersionsSnapshot.end() && D->second.Digest == Digest &&
         !D->second.HadErrors)

clang-tools-extra/clangd/index/SymbolCollector.cpp

 public:
   HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM,
                      const SymbolCollector::Options &Opts)
       : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir) {
   }
 
   // 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;
   }
 
   // Returns a canonical URI for \p Path.
   // 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);
   }
 
   // Gets a canonical include (URI of the header or <header> or "header") for
   // header of \p FID (which should usually be the *expansion* file).
   // This does not account for any per-symbol overrides!
   // Returns "" if includes should not be inserted for this file.
std::optional<llvm::StringRef> getFrameworkHeaderIncludeSpelling(
     else
       *CachedHeaderSpelling =
           llvm::formatv("\"{0}/{1}\"", Framework, HeaderPath->HeaderSubpath)
               .str();
     return llvm::StringRef(*CachedHeaderSpelling);
   }
 
   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();
     // If a file is mapped by canonical headers, use that mapping, regardless
     // of whether it's an otherwise-good header (header guards etc).
     if (Includes) {
       llvm::StringRef Canonical =
           Includes->mapHeader(*SM.getFileEntryRefForID(FID));
       if (!Canonical.empty()) {
         // If we had a mapping, always use it.
         if (Canonical.startswith("<") || Canonical.startswith("\""))
           return Canonical;
         return toURI(Canonical);
       }
     }
     // 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).
       if (Filename.endswith(".inc") || Filename.endswith(".def"))
         // Don't use cache reentrantly due to iterator invalidation.
         return getIncludeHeaderUncached(SM.getFileID(SM.getIncludeLoc(FID)));
       // Conservatively refuse to insert #includes to files without guards.
       return "";
     }
     // Standard case: just insert the file itself.
-    return toURI(FE);
+    return toURI(*FE);
   }
 };
 
 // Return the symbol location of the token at \p TokLoc.
 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;
 
   return Result;
 }
 
 SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {}
if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
     addDefinition(*OriginalDecl, *BasicSymbol);
 
   return true;
 }
 
 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) {
       const auto &Range = MacroRef.Rng;
       bool IsDefinition = MacroRef.IsDefinition;
       Ref R;
       R.Location.Start.setLine(Range.start.line);
       R.Location.Start.setColumn(Range.start.character);
if (I.second)
     I.first->second = Opts.FileFilter(ASTCtx->getSourceManager(), FID);
   return I.first->second;
 }
 
 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);
   }
 }
 
 SymbolID SymbolCollector::getSymbolIDCached(const Decl *D) {
   auto It = DeclToIDCache.try_emplace(D, SymbolID{});

clang-tools-extra/clangd/indexer/IndexerMain.cpp

 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
   IndexActionFactory(IndexFileIn &Result) : Result(Result) {}
 
   std::unique_ptr<FrontendAction> create() override {
     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);
       return Files.insert(*AbsPath).second; // Skip already processed files.
     };
     return createStaticIndexingAction(
         Opts,
         [&](SymbolSlab S) {

clang-tools-extra/clangd/refactor/Tweak.cpp

prepareTweak(StringRef ID, const Tweak::Selection &S,
   }
   return error("tweak ID {0} is invalid", ID);
 }
 
 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))
+  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());
 }
 
 llvm::Expected<Tweak::Effect>
 Tweak::Effect::mainFileEdit(const SourceManager &SM,
                             tooling::Replacements Replacements) {
   auto PathAndEdit = fileEdit(SM, SM.getMainFileID(), std::move(Replacements));