[clang-tools-extra] Preserve original RemoteIndexPath to correctly trim index symbols paths depending on platform (PR #217745)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: Oganesyan Levon (ilev4ik)

<details>
<summary>Changes</summary>

After using version after the fix here: #<!-- -->2646. I faced the next problem: symbols location slashes were converted to posix ones before trimming prefix

---
Full diff: https://github.com/llvm/llvm-project/pull/217745.diff


2 Files Affected:

- (modified) clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp (+20-11) 
- (modified) clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp (+2-7) 


``````````diff
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index db77b8caf5679..a8302fd28776e 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -58,17 +58,22 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
                        llvm::StringRef LocalIndexRoot)
     : Strings(Arena) {
   llvm::StringRef PosixSeparator = get_separator(Style::posix);
+  llvm::StringRef WindowsSeparator = get_separator(Style::windows);
+  const bool IsWindows = is_absolute(RemoteIndexRoot, Style::windows);
+  const bool IsPosix = is_absolute(RemoteIndexRoot, Style::posix);
   if (!RemoteIndexRoot.empty()) {
-    assert(is_absolute(RemoteIndexRoot, Style::posix) ||
-           is_absolute(RemoteIndexRoot, Style::windows));
-    this->RemoteIndexRoot = convert_to_slash(RemoteIndexRoot, Style::windows);
+    assert(IsPosix || IsWindows);
+    this->RemoteIndexRoot = RemoteIndexRoot;
     llvm::StringRef Path(this->RemoteIndexRoot);
-    if (!is_separator(this->RemoteIndexRoot.back(), Style::posix))
+    if (IsPosix && !is_separator(this->RemoteIndexRoot.back(), Style::posix))
       this->RemoteIndexRoot += PosixSeparator;
+    else if (IsWindows &&
+             !is_separator(this->RemoteIndexRoot.back(), Style::windows))
+      this->RemoteIndexRoot += WindowsSeparator;
   }
+
   if (!LocalIndexRoot.empty()) {
-    assert(is_absolute(LocalIndexRoot, Style::posix) ||
-           is_absolute(LocalIndexRoot, Style::windows));
+    assert(IsPosix || IsWindows);
     this->LocalIndexRoot = convert_to_slash(LocalIndexRoot, Style::windows);
     llvm::StringRef Path(this->LocalIndexRoot);
     if (!is_separator(this->LocalIndexRoot.back(), Style::posix))
@@ -264,7 +269,9 @@ FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths) {
     llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-    if (replace_path_prefix(RelativePath, LocalIndexRoot, ""))
+    bool IsWindowsIndex = is_absolute(Path.substr(1), Style::windows);
+    if (replace_path_prefix(RelativePath, LocalIndexRoot, "",
+                            IsWindowsIndex ? Style::windows : Style::posix))
       RPCRequest.add_proximity_paths(
           convert_to_slash(RelativePath, Style::windows));
   }
@@ -396,13 +403,15 @@ llvm::Expected<std::string> Marshaller::uriToRelativePath(llvm::StringRef URI) {
   llvm::SmallString<256> Result = ParsedURI->body();
   llvm::StringRef Path(Result);
   // Check for Windows paths (URI=file:///X:/path => Body=/X:/path)
-  if (is_absolute(Path.substr(1), Style::windows))
+  bool IsWindowsIndex = is_absolute(Path.substr(1), Style::windows);
+  if (IsWindowsIndex)
     Result = Path.drop_front().str();
-  if (!replace_path_prefix(Result, RemoteIndexRoot, ""))
+  if (!replace_path_prefix(Result, RemoteIndexRoot, "",
+                           IsWindowsIndex ? Style::windows : Style::posix))
     return error("File path '{0}' doesn't start with '{1}'.", Result.str(),
                  RemoteIndexRoot);
-  assert(Result == convert_to_slash(Result, Style::windows));
-  return std::string(Result);
+
+  return std::string(convert_to_slash(Result, Style::windows));
 }
 
 clangd::SymbolLocation::Position
diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index fe0dc4a67622c..4ce79cd310c0a 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -465,14 +465,9 @@ TEST(RemoteMarshallingTest, CrossPlatformPathsRoundTrip) {
   Location.End.setLine(3);
   Location.End.setColumn(4);
   // Construct the URI as a Windows machine would have serialized it into the
-  // index: file:///C:/remote/project/lib/File.cpp.
+  // index: file:///C:\remote\project\lib\File.cpp.
   Location.FileURI =
-      Strings
-          .save("file:///" +
-                convert_to_slash(RemoteIndexRoot,
-                                 llvm::sys::path::Style::windows) +
-                "lib/File.cpp")
-          .begin();
+      Strings.save("file:///" + RemoteIndexRoot + "lib\\File.cpp").begin();
   Ref.Location = Location;
 
   auto Serialized = ProtobufMarshaller.toProtobuf(Ref);

``````````

</details>


https://github.com/llvm/llvm-project/pull/217745