[PATCH] D89529: [clangd][remote] Add Windows paths support

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D89529#2334407 <https://reviews.llvm.org/D89529#2334407>, @kbobyrev wrote:

> Hi, thanks for flagging the issue! Could you please give more information where exactly the tests fail (e.g. logs with assertions)? This way I could understand where exactly the failure happens.

With current implementation I could not pass Windows path (e.g. C:\a\b\c) to `Marshaller` constructor because of  `assert(RemoteIndexRoot ==  llvm::sys::path::convert_to_slash(RemoteIndexRoot));`, because on Windows  `llvm::sys::path::convert_to_slash(RemoteIndexRoot)` converts C:\a\b\c to C:/a/b/c

Another problem is that Windows URI paths are not supported without this patch (e.g. URI=file:///X:/path => Body=/X:/path, and /X:/path does not start with X:\path)

The third problem, that without conversion local windows paths to native slashes, mixes-slash paths appears at `path::append()` calls and we again face the problem when some paths does not start with given prefix (but it should)

> I can understand what you're trying to do but the patch introduces the behaviour which is unintended as of right now. The convention is to use POSIX paths everywhere in the Marshalling code since the end result will be a URI anyway. The patch changes it to Windows slashes which is orthogonal to this direction.

Idea is to use native slashes (Windows slashes on Windows) *for local paths only* and UNIX slashes inside the wire.

Just to clarify why I changed `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::posix)` to `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::windows)`:

- `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::posix)` no-op on any platform and does nothing with '\' in Windows paths.
- `llvm::sys::path::convert_to_slash(Result, llvm::sys::path::Style::windows)` replaces '\' with '/' on any platform.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89529/new/

https://reviews.llvm.org/D89529