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: > 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? > Why does not llvm::sys::path::get_separator() return a single char instead of > a null-terminated string? Probably to make composition with `+` easier. But one would need to dig-up the git blame and see if there are any other reasons I suppose, or ask the authors as mine is merely a guess :D 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
