[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:60
+    llvm::StringRef Path(this->RemoteIndexRoot);
+    if (!is_separator(this->RemoteIndexRoot.back(),
+                      llvm::sys::path::Style::posix))
----------------
ArcsinX wrote:
> I know this already commited, but maybe we could use `llvm::sys::path::is_separator` instead of `is_separator` to be consistent with other function calls from `llvm::sys::path` namespace in this file.
> 
> P.S. `llvm::sys::path::is_separator` checks a single char, but `llvm::sys::path::get_separator` returns string. Can not understand why. Could there be a multi-char separator in the future?
Because `llvm::sys::path::get_separator` returns a null terminated string.

> Could there be a multi-char separator in the future?

I would expect the person doing such a change to also update `is_separator`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852