[PATCH] D47406: [ADT] Add `StringRef::rsplit(StringRef Separator)`.

[Zachary Turner via llvm-commits]

lgtm

On Thu, Jun 7, 2018 at 7:35 PM Henry Wong via Phabricator <
reviews at reviews.llvm.org> wrote:

> MTC added inline comments.
>
>
> ================
> Comment at: include/llvm/ADT/StringRef.h:755
> +    /// minimal. If \p Separator is not in the string, then the result is
> a
> +    /// pair (LHS, RHS) where (*this == LHS) and (RHS == "").
> +    ///
> ----------------
> vsk wrote:
> > zturner wrote:
> > > I can't decide if it would make more sense to have the "not found"
> case return a pair `(LHS, RHS)` where `(LHS == "") && (*this == RHS)`?
> What do you think?
> > I think this makes sense, but would make more sense as a follow-up
> commit. I don't think it needs to be a prerequisite for adding an rsplit
> overload.
> Yeah, I also think this is more reasonable.
>
>   # `split()` find the first occurrence, perform a lookup from start to
> end. Given "hello" and separator 'w', the return will be `"hello"` + `""`.
>
>   # `rsplit()` find the last occurrence, perfrom a lookup from end to
> start. Given "hello" and separator'w', the return should be `""` +
> `"hello"`.
>
> If you all think this is more reasonable, I will give another patch to
> implement this. After all, current patch is for adding `rsplit(StringRef)`.
> What do you think?
>
>
>
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D47406
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180607/96cce9ec/attachment.html>