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

Aleksandr Platonov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 19 03:25:09 PDT 2020


ArcsinX added inline comments.


================
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:59
+        RemoteIndexRoot, llvm::sys::path::Style::windows);
+    llvm::StringRef Path(*this->RemoteIndexRoot);
+    if (!Path.endswith(PosixSeparator))
----------------
kbobyrev wrote:
> nit: maybe it's time to change type of `RemoteIndexRoot` field to `llvm::SmallString<256>` and use `!this->RemoteIndexRoot.endswith(PosixSeparator)` instead of additional variable. Not really important for this patch but I should probably do it anyway if it's not changed in this patch.
But `llvm::sys::path::convert_to_slash()` returns `std::string`.  
Could you give me an advice how to copy `std::string` into `llvm::SmallString<256>` here?

E.g. the following code looks strange for me
```
  llvm::Optional<llvm::SmallString<256>> RemoteIndexRoot;

....

    this->RemoteIndexRoot = llvm::SmallString<256>(llvm::sys::path::convert_to_slash(
        RemoteIndexRoot, llvm::sys::path::Style::windows));
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89529



More information about the cfe-commits mailing list