[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC
Kirill Bobyrev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 23 02:18:38 PDT 2020
kbobyrev 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?
I think it'd be better to strip `llvm::sys::path` elsewhere and simply rely on ADL. Spelling `llvm::sys::path::` twice does not really add much value and simply takes more space. I'll send a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89852/new/
https://reviews.llvm.org/D89852
More information about the cfe-commits
mailing list