Skip to content

[clangd] Turn Path and PathRef into classes #136439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Apr 19, 2025

This is the initial step towards clangd/clangd#108.
It introduces the Path and PathRef classes as wrappers for std::string and llvm::StringRef respectively (Path and PathRef were aliases to std::string and llvm::StringRef before). The classes compare and hash case-insensitive based on the platform (based on CLANGD_PATH_CASE_INSENSITIVE - set on Windows and macOS). They can be implicitly constructed from strings but can't be implicitly converted back. Both require .raw() to convert them to the underlying types. One can go from a PathRef to Path using .owned() (that naming is very Rust-y, feel free to suggest something better).

Because of disallowing implicit conversions back to strings, this PR is quite large. My main goal with this is to only introduce the types, but not to refactor code to use them (i.e. it should be functionally the same). Thus, it shouldn't fix the issue yet, but make it possible to adopt the wrappers in multiple parts in parallel. As a first step, by getting rid of as many .raw() calls as possible. I left some todos where I turned a Path(Ref) into a string(ref), because these locations aren't visible when X-Ref searching.

There are many string maps that are actually path maps and would need their keys to be compared and hashed case-insensitively. This isn't possible with a StringMap. I'm not that involved with clangd/LLVM, so I'm not sure what the ideal map type would be.

@llvmbot
Copy link
Member

llvmbot commented Apr 19, 2025

@llvm/pr-subscribers-clangd

Author: nerix (Nerixyz)

Changes

This is the initial step towards clangd/clangd#108.
It introduces the Path and PathRef classes as wrappers for std::string and llvm::StringRef respectively (Path and PathRef were aliases to std::string and llvm::StringRef before). The classes compare and hash case-insensitive based on the platform (based on CLANGD_PATH_CASE_INSENSITIVE - set on Windows and macOS). They can be implicitly constructed from strings but can't be implicitly converted back. Both require .raw() to convert them to the underlying types. One can go from a PathRef to Path using .owned() (that naming is very Rust-y, feel free to suggest something better).

Because of disallowing implicit conversions back to strings, this PR is quite large. My main goal with this is to only introduce the types, but not to refactor code to use them (i.e. it should be functionally the same). Thus, it shouldn't fix the issue yet, but make it possible to adopt the wrappers in multiple parts in parallel. As a first step, by getting rid of as many .raw() calls as possible. I left some todos where I turned a Path(Ref) into a string(ref), because these locations aren't visible when X-Ref searching.

There are many string maps that are actually path maps and would need their keys to be compared and hashed case-insensitively. This isn't possible with a StringMap. I'm not that involved with clangd/LLVM, so I'm not sure what the ideal map type would be.


Patch is 121.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/136439.diff

50 Files Affected:

  • (modified) clang-tools-extra/clangd/ASTSignals.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+38-34)
  • (modified) clang-tools-extra/clangd/CodeComplete.cpp (+17-15)
  • (modified) clang-tools-extra/clangd/ConfigCompile.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/ConfigProvider.cpp (+5-4)
  • (modified) clang-tools-extra/clangd/DraftStore.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/FS.cpp (-6)
  • (modified) clang-tools-extra/clangd/FS.h (-8)
  • (modified) clang-tools-extra/clangd/GlobalCompilationDatabase.cpp (+25-25)
  • (modified) clang-tools-extra/clangd/HeaderSourceSwitch.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/Headers.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/Hover.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/ParsedAST.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/Preamble.cpp (+4-4)
  • (modified) clang-tools-extra/clangd/Protocol.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/Protocol.h (+2-2)
  • (modified) clang-tools-extra/clangd/ScanningProjectModules.cpp (+4-4)
  • (modified) clang-tools-extra/clangd/TUScheduler.cpp (+31-29)
  • (modified) clang-tools-extra/clangd/TidyProvider.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/XRefs.cpp (+15-14)
  • (modified) clang-tools-extra/clangd/index/Background.cpp (+8-6)
  • (modified) clang-tools-extra/clangd/index/Background.h (+1)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexLoader.h (+1-1)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp (+2-2)
  • (modified) clang-tools-extra/clangd/index/FileIndex.cpp (+2-2)
  • (modified) clang-tools-extra/clangd/refactor/Rename.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/refactor/Tweak.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/refactor/Tweak.h (+2-1)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+9-8)
  • (modified) clang-tools-extra/clangd/support/FileCache.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/support/Path.cpp (+125-21)
  • (modified) clang-tools-extra/clangd/support/Path.h (+179-23)
  • (modified) clang-tools-extra/clangd/support/ThreadsafeFS.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/unittests/ASTTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp (+20-23)
  • (modified) clang-tools-extra/clangd/unittests/ClangdTests.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp (+1-6)
  • (modified) clang-tools-extra/clangd/unittests/HeadersTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/PreambleTests.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/TestFS.cpp (+10-11)
  • (modified) clang-tools-extra/clangd/unittests/support/PathTests.cpp (+10-10)
diff --git a/clang-tools-extra/clangd/ASTSignals.cpp b/clang-tools-extra/clangd/ASTSignals.cpp
index cffadb091d557..21647994d259e 100644
--- a/clang-tools-extra/clangd/ASTSignals.cpp
+++ b/clang-tools-extra/clangd/ASTSignals.cpp
@@ -19,7 +19,7 @@ ASTSignals ASTSignals::derive(const ParsedAST &AST) {
   trace::Span Span("ASTSignals::derive");
   ASTSignals Signals;
   Signals.InsertionDirective = preferredIncludeDirective(
-      AST.tuPath(), AST.getLangOpts(),
+      AST.tuPath().raw(), AST.getLangOpts(),
       AST.getIncludeStructure().MainFileIncludes, AST.getLocalTopLevelDecls());
   const SourceManager &SM = AST.getSourceManager();
   findExplicitReferences(
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 1e981825c7c15..35ada432cd5dd 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -926,11 +926,11 @@ void ClangdLSPServer::onDocumentDidClose(
 
   {
     std::lock_guard<std::mutex> Lock(DiagRefMutex);
-    DiagRefMap.erase(File);
+    DiagRefMap.erase(File.raw());
   }
   {
     std::lock_guard<std::mutex> HLock(SemanticTokensMutex);
-    LastSemanticTokens.erase(File);
+    LastSemanticTokens.erase(File.raw());
   }
   // clangd will not send updates for this file anymore, so we empty out the
   // list of diagnostics shown on the client (e.g. in the "Problems" pane of
@@ -1816,7 +1816,7 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
   // Cache DiagRefMap
   {
     std::lock_guard<std::mutex> Lock(DiagRefMutex);
-    DiagRefMap[File] = LocalDiagMap;
+    DiagRefMap[File.raw()] = LocalDiagMap;
   }
 
   // Send a notification to the LSP client.
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 52844129834c3..8ae5aa79674f2 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -88,15 +88,15 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
       indexStdlib(CI, std::move(*Loc));
 
     // FIndex outlives the UpdateIndexCallbacks.
-    auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()),
+    auto Task = [FIndex(FIndex), Path(Path.owned()), Version(Version.str()),
                  ASTCtx(std::move(ASTCtx)), PI(std::move(PI))]() mutable {
       trace::Span Tracer("PreambleIndexing");
-      FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(),
+      FIndex->updatePreamble(Path.raw(), Version, ASTCtx.getASTContext(),
                              ASTCtx.getPreprocessor(), *PI);
     };
 
     if (Tasks) {
-      Tasks->runAsync("Preamble indexing for:" + Path + Version,
+      Tasks->runAsync("Preamble indexing for:" + Path.raw() + Version,
                       std::move(Task));
     } else
       Task();
@@ -262,7 +262,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
     BackgroundIdx = std::make_unique<BackgroundIndex>(
         TFS, CDB,
         BackgroundIndexStorage::createDiskBackedStorageFactory(
-            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+            [&CDB](PathRef File) { return CDB.getProjectInfo(File); }),
         std::move(BGOpts));
     AddIndex(BackgroundIdx.get());
   }
@@ -316,14 +316,14 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
   bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
   // If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
   if (NewFile && BackgroundIdx)
-    BackgroundIdx->boostRelated(File);
+    BackgroundIdx->boostRelated(File.raw());
 }
 
 void ClangdServer::reparseOpenFilesIfNeeded(
     llvm::function_ref<bool(llvm::StringRef File)> Filter) {
   // Reparse only opened files that were modified.
   for (const Path &FilePath : DraftMgr.getActiveFiles())
-    if (Filter(FilePath))
+    if (Filter(FilePath.raw()))
       if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
         addDocument(FilePath, *Draft->Contents, Draft->Version,
                     WantDiagnostics::Auto);
@@ -340,7 +340,7 @@ std::function<Context(PathRef)>
 ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
                                               Callbacks *Publish) {
   if (!Provider)
-    return [](llvm::StringRef) { return Context::current().clone(); };
+    return [](PathRef) { return Context::current().clone(); };
 
   struct Impl {
     const config::Provider *Provider;
@@ -408,8 +408,8 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
   };
 
   // Copyable wrapper.
-  return [I(std::make_shared<Impl>(Provider, Publish))](llvm::StringRef Path) {
-    return (*I)(Path);
+  return [I(std::make_shared<Impl>(Provider, Publish))](PathRef Path) {
+    return (*I)(Path.raw());
   };
 }
 
@@ -426,7 +426,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
   if (!CodeCompleteOpts.Index) // Respect overridden index.
     CodeCompleteOpts.Index = Index;
 
-  auto Task = [Pos, CodeCompleteOpts, File = File.str(), CB = std::move(CB),
+  auto Task = [Pos, CodeCompleteOpts, File = File.owned(), CB = std::move(CB),
                this](llvm::Expected<InputsAndPreamble> IP) mutable {
     if (!IP)
       return CB(IP.takeError());
@@ -442,7 +442,8 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
       SpecFuzzyFind.emplace();
       {
         std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
-        SpecFuzzyFind->CachedReq = CachedCompletionFuzzyFindRequestByFile[File];
+        SpecFuzzyFind->CachedReq =
+            CachedCompletionFuzzyFindRequestByFile[File.raw()];
       }
     }
     ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
@@ -473,7 +474,8 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
       return;
     if (SpecFuzzyFind->NewReq) {
       std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
-      CachedCompletionFuzzyFindRequestByFile[File] = *SpecFuzzyFind->NewReq;
+      CachedCompletionFuzzyFindRequestByFile[File.raw()] =
+          *SpecFuzzyFind->NewReq;
     }
     // Explicitly block until async task completes, this is fine as we've
     // already provided reply to the client and running as a preamble task
@@ -495,7 +497,7 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
                                  MarkupKind DocumentationFormat,
                                  Callback<SignatureHelp> CB) {
 
-  auto Action = [Pos, File = File.str(), CB = std::move(CB),
+  auto Action = [Pos, File = File.owned(), CB = std::move(CB),
                  DocumentationFormat,
                  this](llvm::Expected<InputsAndPreamble> IP) mutable {
     if (!IP)
@@ -540,12 +542,13 @@ void ClangdServer::formatFile(PathRef File, std::optional<Range> Rng,
   }
 
   // Call clang-format.
-  auto Action = [File = File.str(), Code = std::move(*Code),
+  auto Action = [File = File.owned(), Code = std::move(*Code),
                  Ranges = std::vector<tooling::Range>{RequestedRange},
                  CB = std::move(CB), this]() mutable {
-    format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS, true);
+    format::FormatStyle Style =
+        getFormatStyleForFile(File.raw(), Code, TFS, true);
     tooling::Replacements IncludeReplaces =
-        format::sortIncludes(Style, Code, Ranges, File);
+        format::sortIncludes(Style, Code, Ranges, File.raw());
     auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
     if (!Changed)
       return CB(Changed.takeError());
@@ -553,9 +556,9 @@ void ClangdServer::formatFile(PathRef File, std::optional<Range> Rng,
     CB(IncludeReplaces.merge(format::reformat(
         Style, *Changed,
         tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-        File)));
+        File.raw())));
   };
-  WorkScheduler->runQuick("Format", File, std::move(Action));
+  WorkScheduler->runQuick("Format", File.raw(), std::move(Action));
 }
 
 void ClangdServer::formatOnType(PathRef File, Position Pos,
@@ -568,24 +571,24 @@ void ClangdServer::formatOnType(PathRef File, Position Pos,
   llvm::Expected<size_t> CursorPos = positionToOffset(*Code, Pos);
   if (!CursorPos)
     return CB(CursorPos.takeError());
-  auto Action = [File = File.str(), Code = std::move(*Code),
+  auto Action = [File = File.owned(), Code = std::move(*Code),
                  TriggerText = TriggerText.str(), CursorPos = *CursorPos,
                  CB = std::move(CB), this]() mutable {
-    auto Style = getFormatStyleForFile(File, Code, TFS, false);
+    auto Style = getFormatStyleForFile(File.raw(), Code, TFS, false);
     std::vector<TextEdit> Result;
     for (const tooling::Replacement &R :
          formatIncremental(Code, CursorPos, TriggerText, Style))
       Result.push_back(replacementToEdit(Code, R));
     return CB(Result);
   };
-  WorkScheduler->runQuick("FormatOnType", File, std::move(Action));
+  WorkScheduler->runQuick("FormatOnType", File.raw(), std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
                                  std::optional<std::string> NewName,
                                  const RenameOptions &RenameOpts,
                                  Callback<RenameResult> CB) {
-  auto Action = [Pos, File = File.str(), CB = std::move(CB),
+  auto Action = [Pos, File = File.owned(), CB = std::move(CB),
                  NewName = std::move(NewName),
                  RenameOpts](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
@@ -594,7 +597,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
     // only need main-file references
     auto Results =
         clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"),
-                        InpAST->AST, File, /*FS=*/nullptr,
+                        InpAST->AST, File.raw(), /*FS=*/nullptr,
                         /*Index=*/nullptr, RenameOpts});
     if (!Results) {
       // LSP says to return null on failure, but that will result in a generic
@@ -610,7 +613,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                           const RenameOptions &Opts,
                           Callback<RenameResult> CB) {
-  auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
+  auto Action = [File = File.owned(), NewName = NewName.str(), Pos, Opts,
                  CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     // Tracks number of files edited per invocation.
@@ -618,13 +621,13 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                                                trace::Metric::Distribution);
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto R = clangd::rename({Pos, NewName, InpAST->AST, File,
+    auto R = clangd::rename({Pos, NewName, InpAST->AST, File.raw(),
                              DirtyFS->view(std::nullopt), Index, Opts});
     if (!R)
       return CB(R.takeError());
 
     if (Opts.WantFormat) {
-      auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
+      auto Style = getFormatStyleForFile(File.raw(), InpAST->Inputs.Contents,
                                          *InpAST->Inputs.TFS, false);
       llvm::Error Err = llvm::Error::success();
       for (auto &E : R->GlobalChanges)
@@ -756,7 +759,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   static constexpr trace::Metric TweakFailed(
       "tweak_failed", trace::Metric::Counter, "tweak_id");
   TweakAttempt.record(1, TweakID);
-  auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
+  auto Action = [File = File.owned(), Sel, TweakID = TweakID.str(),
                  CB = std::move(CB),
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
@@ -782,7 +785,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
       for (auto &It : (*Effect)->ApplyEdits) {
         Edit &E = It.second;
         format::FormatStyle Style =
-            getFormatStyleForFile(File, E.InitialCode, TFS, false);
+            getFormatStyleForFile(File.raw(), E.InitialCode, TFS, false);
         if (llvm::Error Err = reformatEdit(E, Style))
           elog("Failed to format {0}: {1}", It.first(), std::move(Err));
       }
@@ -817,7 +820,7 @@ void ClangdServer::switchSourceHeader(
   if (auto CorrespondingFile =
           getCorrespondingHeaderOrSource(Path, TFS.view(std::nullopt)))
     return CB(std::move(CorrespondingFile));
-  auto Action = [Path = Path.str(), CB = std::move(CB),
+  auto Action = [Path = Path.owned(), CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -840,12 +843,12 @@ void ClangdServer::findDocumentHighlights(
 
 void ClangdServer::findHover(PathRef File, Position Pos,
                              Callback<std::optional<HoverInfo>> CB) {
-  auto Action = [File = File.str(), Pos, CB = std::move(CB),
+  auto Action = [File = File.owned(), Pos, CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
     format::FormatStyle Style = getFormatStyleForFile(
-        File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
+        File.raw(), InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
     CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
   };
 
@@ -855,7 +858,8 @@ void ClangdServer::findHover(PathRef File, Position Pos,
 void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve,
                                  TypeHierarchyDirection Direction,
                                  Callback<std::vector<TypeHierarchyItem>> CB) {
-  auto Action = [File = File.str(), Pos, Resolve, Direction, CB = std::move(CB),
+  auto Action = [File = File.owned(), Pos, Resolve, Direction,
+                 CB = std::move(CB),
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -894,7 +898,7 @@ void ClangdServer::resolveTypeHierarchy(
 
 void ClangdServer::prepareCallHierarchy(
     PathRef File, Position Pos, Callback<std::vector<CallHierarchyItem>> CB) {
-  auto Action = [File = File.str(), Pos,
+  auto Action = [File = File.owned(), Pos,
                  CB = std::move(CB)](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -976,7 +980,7 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
   WorkScheduler->runQuick("FoldingRanges", File, std::move(Action));
 }
 
-void ClangdServer::findType(llvm::StringRef File, Position Pos,
+void ClangdServer::findType(PathRef File, Position Pos,
                             Callback<std::vector<LocatedSymbol>> CB) {
   auto Action = [Pos, CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0eb196fbad46a..ebbcea5f67bbf 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1384,14 +1384,14 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   CI->getLangOpts().DelayedTemplateParsing = false;
   // Setup code completion.
   FrontendOpts.CodeCompleteOpts = Options;
-  FrontendOpts.CodeCompletionAt.FileName = std::string(Input.FileName);
+  FrontendOpts.CodeCompletionAt.FileName = Input.FileName.owned().raw();
   std::tie(FrontendOpts.CodeCompletionAt.Line,
            FrontendOpts.CodeCompletionAt.Column) =
       offsetToClangLineColumn(Input.ParseInput.Contents, Input.Offset);
 
   std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
       llvm::MemoryBuffer::getMemBuffer(Input.ParseInput.Contents,
-                                       Input.FileName);
+                                       Input.FileName.raw());
   // The diagnostic options must be set before creating a CompilerInstance.
   CI->getDiagnosticOpts().IgnoreWarnings = true;
   // We reuse the preamble whether it's valid or not. This is a
@@ -1649,7 +1649,7 @@ class CodeCompleteFlow {
       assert(Recorder && "Recorder is not set");
       CCContextKind = Recorder->CCContext.getKind();
       IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
-      auto Style = getFormatStyleForFile(SemaCCInput.FileName,
+      auto Style = getFormatStyleForFile(SemaCCInput.FileName.raw(),
                                          SemaCCInput.ParseInput.Contents,
                                          *SemaCCInput.ParseInput.TFS, false);
       const auto NextToken = findTokenAfterCompletionPoint(
@@ -1660,7 +1660,7 @@ class CodeCompleteFlow {
       // If preprocessor was run, inclusions from preprocessor callback should
       // already be added to Includes.
       Inserter.emplace(
-          SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
+          SemaCCInput.FileName.raw(), SemaCCInput.ParseInput.Contents, Style,
           SemaCCInput.ParseInput.CompileCommand.Directory,
           &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo(),
           Config::current().Style.QuotedHeaders,
@@ -1741,12 +1741,12 @@ class CodeCompleteFlow {
     ReplacedRange.start.character -= HeuristicPrefix.Name.size();
 
     llvm::StringMap<SourceParams> ProxSources;
-    ProxSources[FileName].Cost = 0;
+    ProxSources[FileName.raw()].Cost = 0;
     FileProximity.emplace(ProxSources);
 
-    auto Style = getFormatStyleForFile(FileName, Content, TFS, false);
+    auto Style = getFormatStyleForFile(FileName.raw(), Content, TFS, false);
     // This will only insert verbatim headers.
-    Inserter.emplace(FileName, Content, Style,
+    Inserter.emplace(FileName.raw(), Content, Style,
                      /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
                      Config::current().Style.QuotedHeaders,
                      Config::current().Style.AngledHeaders);
@@ -1917,7 +1917,7 @@ class CodeCompleteFlow {
     Req.Scopes = QueryScopes;
     Req.AnyScope = AllScopes;
     // FIXME: we should send multiple weighted paths here.
-    Req.ProximityPaths.push_back(std::string(FileName));
+    Req.ProximityPaths.push_back(FileName.owned().raw());
     if (PreferredType)
       Req.PreferredTypes.push_back(std::string(PreferredType->raw()));
     vlog("Code complete: fuzzyFind({0:2})", toJSON(Req));
@@ -1972,8 +1972,9 @@ class CodeCompleteFlow {
         assert(IdentifierResult);
         C.Name = IdentifierResult->Name;
       }
-      if (auto OverloadSet = C.overloadSet(
-              Opts, FileName, Inserter ? &*Inserter : nullptr, CCContextKind)) {
+      if (auto OverloadSet =
+              C.overloadSet(Opts, FileName.raw(),
+                            Inserter ? &*Inserter : nullptr, CCContextKind)) {
         auto Ret = BundleLookup.try_emplace(OverloadSet, Bundles.size());
         if (Ret.second)
           Bundles.emplace_back();
@@ -2140,8 +2141,9 @@ class CodeCompleteFlow {
                           : nullptr;
       if (!Builder)
         Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
-                        Item, SemaCCS, AccessibleScopes, *Inserter, FileName,
-                        CCContextKind, Opts, IsUsingDeclaration, NextTokenKind);
+                        Item, SemaCCS, AccessibleScopes, *Inserter,
+                        FileName.raw(), CCContextKind, Opts, IsUsingDeclaration,
+                        NextTokenKind);
       else
         Builder->add(Item, SemaCCS, CCContextKind);
     }
@@ -2214,7 +2216,7 @@ CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset,
   semaCodeComplete(
       std::make_unique<ParamNameCollector>(Options, ParamNames), Options,
       {FileName, Offset, *Preamble,
-       PreamblePatch::createFullPatch(FileName, ParseInput, *Preamble),
+       PreamblePatch::createFullPatch(FileName.raw(), ParseInput, *Preamble),
        ParseInput});
   if (ParamNames.empty())
     return CodeCompleteResult();
@@ -2288,7 +2290,7 @@ CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Apr 19, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: nerix (Nerixyz)

Changes

This is the initial step towards clangd/clangd#108.
It introduces the Path and PathRef classes as wrappers for std::string and llvm::StringRef respectively (Path and PathRef were aliases to std::string and llvm::StringRef before). The classes compare and hash case-insensitive based on the platform (based on CLANGD_PATH_CASE_INSENSITIVE - set on Windows and macOS). They can be implicitly constructed from strings but can't be implicitly converted back. Both require .raw() to convert them to the underlying types. One can go from a PathRef to Path using .owned() (that naming is very Rust-y, feel free to suggest something better).

Because of disallowing implicit conversions back to strings, this PR is quite large. My main goal with this is to only introduce the types, but not to refactor code to use them (i.e. it should be functionally the same). Thus, it shouldn't fix the issue yet, but make it possible to adopt the wrappers in multiple parts in parallel. As a first step, by getting rid of as many .raw() calls as possible. I left some todos where I turned a Path(Ref) into a string(ref), because these locations aren't visible when X-Ref searching.

There are many string maps that are actually path maps and would need their keys to be compared and hashed case-insensitively. This isn't possible with a StringMap. I'm not that involved with clangd/LLVM, so I'm not sure what the ideal map type would be.


Patch is 121.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/136439.diff

50 Files Affected:

  • (modified) clang-tools-extra/clangd/ASTSignals.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+38-34)
  • (modified) clang-tools-extra/clangd/CodeComplete.cpp (+17-15)
  • (modified) clang-tools-extra/clangd/ConfigCompile.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/ConfigProvider.cpp (+5-4)
  • (modified) clang-tools-extra/clangd/DraftStore.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/FS.cpp (-6)
  • (modified) clang-tools-extra/clangd/FS.h (-8)
  • (modified) clang-tools-extra/clangd/GlobalCompilationDatabase.cpp (+25-25)
  • (modified) clang-tools-extra/clangd/HeaderSourceSwitch.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/Headers.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/Hover.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/ParsedAST.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/Preamble.cpp (+4-4)
  • (modified) clang-tools-extra/clangd/Protocol.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/Protocol.h (+2-2)
  • (modified) clang-tools-extra/clangd/ScanningProjectModules.cpp (+4-4)
  • (modified) clang-tools-extra/clangd/TUScheduler.cpp (+31-29)
  • (modified) clang-tools-extra/clangd/TidyProvider.cpp (+7-7)
  • (modified) clang-tools-extra/clangd/XRefs.cpp (+15-14)
  • (modified) clang-tools-extra/clangd/index/Background.cpp (+8-6)
  • (modified) clang-tools-extra/clangd/index/Background.h (+1)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexLoader.h (+1-1)
  • (modified) clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp (+2-2)
  • (modified) clang-tools-extra/clangd/index/FileIndex.cpp (+2-2)
  • (modified) clang-tools-extra/clangd/refactor/Rename.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/refactor/Tweak.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/refactor/Tweak.h (+2-1)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp (+3-2)
  • (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+9-8)
  • (modified) clang-tools-extra/clangd/support/FileCache.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/support/Path.cpp (+125-21)
  • (modified) clang-tools-extra/clangd/support/Path.h (+179-23)
  • (modified) clang-tools-extra/clangd/support/ThreadsafeFS.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/unittests/ASTTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp (+20-23)
  • (modified) clang-tools-extra/clangd/unittests/ClangdTests.cpp (+8-8)
  • (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp (+1-6)
  • (modified) clang-tools-extra/clangd/unittests/HeadersTests.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/unittests/PreambleTests.cpp (+5-5)
  • (modified) clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp (+3-3)
  • (modified) clang-tools-extra/clangd/unittests/TestFS.cpp (+10-11)
  • (modified) clang-tools-extra/clangd/unittests/support/PathTests.cpp (+10-10)
diff --git a/clang-tools-extra/clangd/ASTSignals.cpp b/clang-tools-extra/clangd/ASTSignals.cpp
index cffadb091d557..21647994d259e 100644
--- a/clang-tools-extra/clangd/ASTSignals.cpp
+++ b/clang-tools-extra/clangd/ASTSignals.cpp
@@ -19,7 +19,7 @@ ASTSignals ASTSignals::derive(const ParsedAST &AST) {
   trace::Span Span("ASTSignals::derive");
   ASTSignals Signals;
   Signals.InsertionDirective = preferredIncludeDirective(
-      AST.tuPath(), AST.getLangOpts(),
+      AST.tuPath().raw(), AST.getLangOpts(),
       AST.getIncludeStructure().MainFileIncludes, AST.getLocalTopLevelDecls());
   const SourceManager &SM = AST.getSourceManager();
   findExplicitReferences(
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 1e981825c7c15..35ada432cd5dd 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -926,11 +926,11 @@ void ClangdLSPServer::onDocumentDidClose(
 
   {
     std::lock_guard<std::mutex> Lock(DiagRefMutex);
-    DiagRefMap.erase(File);
+    DiagRefMap.erase(File.raw());
   }
   {
     std::lock_guard<std::mutex> HLock(SemanticTokensMutex);
-    LastSemanticTokens.erase(File);
+    LastSemanticTokens.erase(File.raw());
   }
   // clangd will not send updates for this file anymore, so we empty out the
   // list of diagnostics shown on the client (e.g. in the "Problems" pane of
@@ -1816,7 +1816,7 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
   // Cache DiagRefMap
   {
     std::lock_guard<std::mutex> Lock(DiagRefMutex);
-    DiagRefMap[File] = LocalDiagMap;
+    DiagRefMap[File.raw()] = LocalDiagMap;
   }
 
   // Send a notification to the LSP client.
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 52844129834c3..8ae5aa79674f2 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -88,15 +88,15 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
       indexStdlib(CI, std::move(*Loc));
 
     // FIndex outlives the UpdateIndexCallbacks.
-    auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()),
+    auto Task = [FIndex(FIndex), Path(Path.owned()), Version(Version.str()),
                  ASTCtx(std::move(ASTCtx)), PI(std::move(PI))]() mutable {
       trace::Span Tracer("PreambleIndexing");
-      FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(),
+      FIndex->updatePreamble(Path.raw(), Version, ASTCtx.getASTContext(),
                              ASTCtx.getPreprocessor(), *PI);
     };
 
     if (Tasks) {
-      Tasks->runAsync("Preamble indexing for:" + Path + Version,
+      Tasks->runAsync("Preamble indexing for:" + Path.raw() + Version,
                       std::move(Task));
     } else
       Task();
@@ -262,7 +262,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
     BackgroundIdx = std::make_unique<BackgroundIndex>(
         TFS, CDB,
         BackgroundIndexStorage::createDiskBackedStorageFactory(
-            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+            [&CDB](PathRef File) { return CDB.getProjectInfo(File); }),
         std::move(BGOpts));
     AddIndex(BackgroundIdx.get());
   }
@@ -316,14 +316,14 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
   bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
   // If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
   if (NewFile && BackgroundIdx)
-    BackgroundIdx->boostRelated(File);
+    BackgroundIdx->boostRelated(File.raw());
 }
 
 void ClangdServer::reparseOpenFilesIfNeeded(
     llvm::function_ref<bool(llvm::StringRef File)> Filter) {
   // Reparse only opened files that were modified.
   for (const Path &FilePath : DraftMgr.getActiveFiles())
-    if (Filter(FilePath))
+    if (Filter(FilePath.raw()))
       if (auto Draft = DraftMgr.getDraft(FilePath)) // else disappeared in race?
         addDocument(FilePath, *Draft->Contents, Draft->Version,
                     WantDiagnostics::Auto);
@@ -340,7 +340,7 @@ std::function<Context(PathRef)>
 ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
                                               Callbacks *Publish) {
   if (!Provider)
-    return [](llvm::StringRef) { return Context::current().clone(); };
+    return [](PathRef) { return Context::current().clone(); };
 
   struct Impl {
     const config::Provider *Provider;
@@ -408,8 +408,8 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider,
   };
 
   // Copyable wrapper.
-  return [I(std::make_shared<Impl>(Provider, Publish))](llvm::StringRef Path) {
-    return (*I)(Path);
+  return [I(std::make_shared<Impl>(Provider, Publish))](PathRef Path) {
+    return (*I)(Path.raw());
   };
 }
 
@@ -426,7 +426,7 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
   if (!CodeCompleteOpts.Index) // Respect overridden index.
     CodeCompleteOpts.Index = Index;
 
-  auto Task = [Pos, CodeCompleteOpts, File = File.str(), CB = std::move(CB),
+  auto Task = [Pos, CodeCompleteOpts, File = File.owned(), CB = std::move(CB),
                this](llvm::Expected<InputsAndPreamble> IP) mutable {
     if (!IP)
       return CB(IP.takeError());
@@ -442,7 +442,8 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
       SpecFuzzyFind.emplace();
       {
         std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
-        SpecFuzzyFind->CachedReq = CachedCompletionFuzzyFindRequestByFile[File];
+        SpecFuzzyFind->CachedReq =
+            CachedCompletionFuzzyFindRequestByFile[File.raw()];
       }
     }
     ParseInputs ParseInput{IP->Command, &getHeaderFS(), IP->Contents.str()};
@@ -473,7 +474,8 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
       return;
     if (SpecFuzzyFind->NewReq) {
       std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
-      CachedCompletionFuzzyFindRequestByFile[File] = *SpecFuzzyFind->NewReq;
+      CachedCompletionFuzzyFindRequestByFile[File.raw()] =
+          *SpecFuzzyFind->NewReq;
     }
     // Explicitly block until async task completes, this is fine as we've
     // already provided reply to the client and running as a preamble task
@@ -495,7 +497,7 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
                                  MarkupKind DocumentationFormat,
                                  Callback<SignatureHelp> CB) {
 
-  auto Action = [Pos, File = File.str(), CB = std::move(CB),
+  auto Action = [Pos, File = File.owned(), CB = std::move(CB),
                  DocumentationFormat,
                  this](llvm::Expected<InputsAndPreamble> IP) mutable {
     if (!IP)
@@ -540,12 +542,13 @@ void ClangdServer::formatFile(PathRef File, std::optional<Range> Rng,
   }
 
   // Call clang-format.
-  auto Action = [File = File.str(), Code = std::move(*Code),
+  auto Action = [File = File.owned(), Code = std::move(*Code),
                  Ranges = std::vector<tooling::Range>{RequestedRange},
                  CB = std::move(CB), this]() mutable {
-    format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS, true);
+    format::FormatStyle Style =
+        getFormatStyleForFile(File.raw(), Code, TFS, true);
     tooling::Replacements IncludeReplaces =
-        format::sortIncludes(Style, Code, Ranges, File);
+        format::sortIncludes(Style, Code, Ranges, File.raw());
     auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
     if (!Changed)
       return CB(Changed.takeError());
@@ -553,9 +556,9 @@ void ClangdServer::formatFile(PathRef File, std::optional<Range> Rng,
     CB(IncludeReplaces.merge(format::reformat(
         Style, *Changed,
         tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
-        File)));
+        File.raw())));
   };
-  WorkScheduler->runQuick("Format", File, std::move(Action));
+  WorkScheduler->runQuick("Format", File.raw(), std::move(Action));
 }
 
 void ClangdServer::formatOnType(PathRef File, Position Pos,
@@ -568,24 +571,24 @@ void ClangdServer::formatOnType(PathRef File, Position Pos,
   llvm::Expected<size_t> CursorPos = positionToOffset(*Code, Pos);
   if (!CursorPos)
     return CB(CursorPos.takeError());
-  auto Action = [File = File.str(), Code = std::move(*Code),
+  auto Action = [File = File.owned(), Code = std::move(*Code),
                  TriggerText = TriggerText.str(), CursorPos = *CursorPos,
                  CB = std::move(CB), this]() mutable {
-    auto Style = getFormatStyleForFile(File, Code, TFS, false);
+    auto Style = getFormatStyleForFile(File.raw(), Code, TFS, false);
     std::vector<TextEdit> Result;
     for (const tooling::Replacement &R :
          formatIncremental(Code, CursorPos, TriggerText, Style))
       Result.push_back(replacementToEdit(Code, R));
     return CB(Result);
   };
-  WorkScheduler->runQuick("FormatOnType", File, std::move(Action));
+  WorkScheduler->runQuick("FormatOnType", File.raw(), std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
                                  std::optional<std::string> NewName,
                                  const RenameOptions &RenameOpts,
                                  Callback<RenameResult> CB) {
-  auto Action = [Pos, File = File.str(), CB = std::move(CB),
+  auto Action = [Pos, File = File.owned(), CB = std::move(CB),
                  NewName = std::move(NewName),
                  RenameOpts](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
@@ -594,7 +597,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
     // only need main-file references
     auto Results =
         clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"),
-                        InpAST->AST, File, /*FS=*/nullptr,
+                        InpAST->AST, File.raw(), /*FS=*/nullptr,
                         /*Index=*/nullptr, RenameOpts});
     if (!Results) {
       // LSP says to return null on failure, but that will result in a generic
@@ -610,7 +613,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
 void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                           const RenameOptions &Opts,
                           Callback<RenameResult> CB) {
-  auto Action = [File = File.str(), NewName = NewName.str(), Pos, Opts,
+  auto Action = [File = File.owned(), NewName = NewName.str(), Pos, Opts,
                  CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     // Tracks number of files edited per invocation.
@@ -618,13 +621,13 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                                                trace::Metric::Distribution);
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto R = clangd::rename({Pos, NewName, InpAST->AST, File,
+    auto R = clangd::rename({Pos, NewName, InpAST->AST, File.raw(),
                              DirtyFS->view(std::nullopt), Index, Opts});
     if (!R)
       return CB(R.takeError());
 
     if (Opts.WantFormat) {
-      auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
+      auto Style = getFormatStyleForFile(File.raw(), InpAST->Inputs.Contents,
                                          *InpAST->Inputs.TFS, false);
       llvm::Error Err = llvm::Error::success();
       for (auto &E : R->GlobalChanges)
@@ -756,7 +759,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   static constexpr trace::Metric TweakFailed(
       "tweak_failed", trace::Metric::Counter, "tweak_id");
   TweakAttempt.record(1, TweakID);
-  auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
+  auto Action = [File = File.owned(), Sel, TweakID = TweakID.str(),
                  CB = std::move(CB),
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
@@ -782,7 +785,7 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
       for (auto &It : (*Effect)->ApplyEdits) {
         Edit &E = It.second;
         format::FormatStyle Style =
-            getFormatStyleForFile(File, E.InitialCode, TFS, false);
+            getFormatStyleForFile(File.raw(), E.InitialCode, TFS, false);
         if (llvm::Error Err = reformatEdit(E, Style))
           elog("Failed to format {0}: {1}", It.first(), std::move(Err));
       }
@@ -817,7 +820,7 @@ void ClangdServer::switchSourceHeader(
   if (auto CorrespondingFile =
           getCorrespondingHeaderOrSource(Path, TFS.view(std::nullopt)))
     return CB(std::move(CorrespondingFile));
-  auto Action = [Path = Path.str(), CB = std::move(CB),
+  auto Action = [Path = Path.owned(), CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -840,12 +843,12 @@ void ClangdServer::findDocumentHighlights(
 
 void ClangdServer::findHover(PathRef File, Position Pos,
                              Callback<std::optional<HoverInfo>> CB) {
-  auto Action = [File = File.str(), Pos, CB = std::move(CB),
+  auto Action = [File = File.owned(), Pos, CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
     format::FormatStyle Style = getFormatStyleForFile(
-        File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
+        File.raw(), InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
     CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
   };
 
@@ -855,7 +858,8 @@ void ClangdServer::findHover(PathRef File, Position Pos,
 void ClangdServer::typeHierarchy(PathRef File, Position Pos, int Resolve,
                                  TypeHierarchyDirection Direction,
                                  Callback<std::vector<TypeHierarchyItem>> CB) {
-  auto Action = [File = File.str(), Pos, Resolve, Direction, CB = std::move(CB),
+  auto Action = [File = File.owned(), Pos, Resolve, Direction,
+                 CB = std::move(CB),
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -894,7 +898,7 @@ void ClangdServer::resolveTypeHierarchy(
 
 void ClangdServer::prepareCallHierarchy(
     PathRef File, Position Pos, Callback<std::vector<CallHierarchyItem>> CB) {
-  auto Action = [File = File.str(), Pos,
+  auto Action = [File = File.owned(), Pos,
                  CB = std::move(CB)](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
@@ -976,7 +980,7 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
   WorkScheduler->runQuick("FoldingRanges", File, std::move(Action));
 }
 
-void ClangdServer::findType(llvm::StringRef File, Position Pos,
+void ClangdServer::findType(PathRef File, Position Pos,
                             Callback<std::vector<LocatedSymbol>> CB) {
   auto Action = [Pos, CB = std::move(CB),
                  this](llvm::Expected<InputsAndAST> InpAST) mutable {
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0eb196fbad46a..ebbcea5f67bbf 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1384,14 +1384,14 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   CI->getLangOpts().DelayedTemplateParsing = false;
   // Setup code completion.
   FrontendOpts.CodeCompleteOpts = Options;
-  FrontendOpts.CodeCompletionAt.FileName = std::string(Input.FileName);
+  FrontendOpts.CodeCompletionAt.FileName = Input.FileName.owned().raw();
   std::tie(FrontendOpts.CodeCompletionAt.Line,
            FrontendOpts.CodeCompletionAt.Column) =
       offsetToClangLineColumn(Input.ParseInput.Contents, Input.Offset);
 
   std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
       llvm::MemoryBuffer::getMemBuffer(Input.ParseInput.Contents,
-                                       Input.FileName);
+                                       Input.FileName.raw());
   // The diagnostic options must be set before creating a CompilerInstance.
   CI->getDiagnosticOpts().IgnoreWarnings = true;
   // We reuse the preamble whether it's valid or not. This is a
@@ -1649,7 +1649,7 @@ class CodeCompleteFlow {
       assert(Recorder && "Recorder is not set");
       CCContextKind = Recorder->CCContext.getKind();
       IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
-      auto Style = getFormatStyleForFile(SemaCCInput.FileName,
+      auto Style = getFormatStyleForFile(SemaCCInput.FileName.raw(),
                                          SemaCCInput.ParseInput.Contents,
                                          *SemaCCInput.ParseInput.TFS, false);
       const auto NextToken = findTokenAfterCompletionPoint(
@@ -1660,7 +1660,7 @@ class CodeCompleteFlow {
       // If preprocessor was run, inclusions from preprocessor callback should
       // already be added to Includes.
       Inserter.emplace(
-          SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
+          SemaCCInput.FileName.raw(), SemaCCInput.ParseInput.Contents, Style,
           SemaCCInput.ParseInput.CompileCommand.Directory,
           &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo(),
           Config::current().Style.QuotedHeaders,
@@ -1741,12 +1741,12 @@ class CodeCompleteFlow {
     ReplacedRange.start.character -= HeuristicPrefix.Name.size();
 
     llvm::StringMap<SourceParams> ProxSources;
-    ProxSources[FileName].Cost = 0;
+    ProxSources[FileName.raw()].Cost = 0;
     FileProximity.emplace(ProxSources);
 
-    auto Style = getFormatStyleForFile(FileName, Content, TFS, false);
+    auto Style = getFormatStyleForFile(FileName.raw(), Content, TFS, false);
     // This will only insert verbatim headers.
-    Inserter.emplace(FileName, Content, Style,
+    Inserter.emplace(FileName.raw(), Content, Style,
                      /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
                      Config::current().Style.QuotedHeaders,
                      Config::current().Style.AngledHeaders);
@@ -1917,7 +1917,7 @@ class CodeCompleteFlow {
     Req.Scopes = QueryScopes;
     Req.AnyScope = AllScopes;
     // FIXME: we should send multiple weighted paths here.
-    Req.ProximityPaths.push_back(std::string(FileName));
+    Req.ProximityPaths.push_back(FileName.owned().raw());
     if (PreferredType)
       Req.PreferredTypes.push_back(std::string(PreferredType->raw()));
     vlog("Code complete: fuzzyFind({0:2})", toJSON(Req));
@@ -1972,8 +1972,9 @@ class CodeCompleteFlow {
         assert(IdentifierResult);
         C.Name = IdentifierResult->Name;
       }
-      if (auto OverloadSet = C.overloadSet(
-              Opts, FileName, Inserter ? &*Inserter : nullptr, CCContextKind)) {
+      if (auto OverloadSet =
+              C.overloadSet(Opts, FileName.raw(),
+                            Inserter ? &*Inserter : nullptr, CCContextKind)) {
         auto Ret = BundleLookup.try_emplace(OverloadSet, Bundles.size());
         if (Ret.second)
           Bundles.emplace_back();
@@ -2140,8 +2141,9 @@ class CodeCompleteFlow {
                           : nullptr;
       if (!Builder)
         Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
-                        Item, SemaCCS, AccessibleScopes, *Inserter, FileName,
-                        CCContextKind, Opts, IsUsingDeclaration, NextTokenKind);
+                        Item, SemaCCS, AccessibleScopes, *Inserter,
+                        FileName.raw(), CCContextKind, Opts, IsUsingDeclaration,
+                        NextTokenKind);
       else
         Builder->add(Item, SemaCCS, CCContextKind);
     }
@@ -2214,7 +2216,7 @@ CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset,
   semaCodeComplete(
       std::make_unique<ParamNameCollector>(Options, ParamNames), Options,
       {FileName, Offset, *Preamble,
-       PreamblePatch::createFullPatch(FileName, ParseInput, *Preamble),
+       PreamblePatch::createFullPatch(FileName.raw(), ParseInput, *Preamble),
        ParseInput});
   if (ParamNames.empty())
     return CodeCompleteResult();
@@ -2288,7 +2290,7 @@ CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
...
[truncated]

Copy link

github-actions bot commented Apr 19, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Nerixyz Nerixyz force-pushed the refactor/path-class branch from fdab9b9 to c4c1e7a Compare April 19, 2025 17:38
@Nerixyz Nerixyz force-pushed the refactor/path-class branch from c4c1e7a to 9fc8237 Compare April 19, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants