[PATCH] D47973: [ADT] Change the behavior of `StringRef::rsplit()` when the separator is not found.

[Henry Wong via Phabricator via llvm-commits]

MTC added a comment.

In https://reviews.llvm.org/D47973#1127309, @vsk wrote:

> Thanks for the patch. The StringRef change itself looks fine, but please make sure to update the callsites in-tree (e.g in llvm, clang, lldb etc).


Thanks for your reminder, vsk! Sorry for my negligence, I thought these corner cases were definitely included in the test case, so after the test was successful, I thought there was no potential problem.

After I re-examined some of the usages of `StringRef::rsplit()`, I think I need to abandon this patch.

There are three reasons for this:

- With some usages, the string may need to be `rsplit()` through multiple `separator`. After splitting using the first `separator`, we can always use `sample_string.rsplit(separator).first` for the next split stage. However for this patch, we have to use `sample_sring.rsplit(separator).first` if the `separator` is in `sample_string`, and use `sample_string.rsplit(separator).second` if the `separator` is not in `sample_string`. The code is not clean!
- Changing the behavior of existing libraries is dangerous especially it is difficult for me to figure out all the code logic of the `StringRef::rsplit()` usages.
- Like the `rsplit()` in other programming languages, e.g python. `rsplit()` in python splits string from the right at the specified separator and returns a list of strings, and `rsplit()` guarantee `list[0]` always have values even if the `separator` isn't in the string. So for `StringRef::rsplit()`, even though `StringRef::rsplit()` returns a pair instead of list, we should guarantee that `pair.first` always has value for subsequent use.

Sorry for the lack of thinking about this patch!

What do you think, @xbolva00, @vsk!


Repository:
  rL LLVM

https://reviews.llvm.org/D47973