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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
