ArcsinX 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)) ---------------- kbobyrev wrote: > kadircet wrote: > > 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`. > 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. > Because llvm::sys::path::get_separator returns a null terminated string. Why does not `llvm::sys::path::get_separator()` return a single char instead of a null-terminated string? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89852/new/ https://reviews.llvm.org/D89852 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
