For StringRef::split(), it performs the search from the front to the back to find the first occurence of the separator, so when the separator is not found, returning <*this, ""> makes sense. For SrtingRef::rsplit(), it performs the search from the back to the front to find the "first" occurence of the separator too, so when the separator is not found, returning <"", *this> makes more sense.
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 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!