basic_string_view's non-member begin/end should take basic_string_view by value

[CaseyCarter]

Description

... as the working draft requires. Test coverage failed to detect this issue due to an overload resolution bug in MSVC.

Drive-by: Remove the "accepts rvalues" bit from the comment in begin which caused the confusion that gave rise to #104. Hopefully it is now glaringly obvious that begin and end accept both lvalues and rvalues.

Resolves #104.

[This is a replay of Microsoft-internal PR 203374.]

Checklist:

• I understand README.md.
• If this is a feature addition, that feature has been voted into the C++
  Working Draft. (N/A)
• Any code files edited have been processed by clang-format 8.0.1.
  (The version is important because clang-format's behavior sometimes changes.)
• Identifiers in any product code changes are properly _Ugly as per
  https://eel.is/c++draft/lex.name#3.1 .
• Identifiers in test code changes are not _Ugly.
• Test code includes the correct headers as per the Standard, not just
  what happens to compile.
• The STL builds and test harnesses have passed (must be manually verified
  by an STL maintainer before CI is online, leave this unchecked for initial
  submission).
• This change introduces no known ABI breaks (adding members, renaming
  members, adding virtual functions, changing whether a type is an aggregate or
  trivially copyable, etc.). If unsure, leave this box unchecked and ask a
  maintainer for help.

[StephanTLavavej]

You could pass by const value, as other functions in this file do. OK to merge as-is.

[miscco]

Out of curiosity why take it by value and not rvalue_reference?

The lvalue_reference case is already covered by the classical std::begin() from <iterator>. So what do you gain by not writing basic_string_view&& _Right?

[shuffle-c]

Out of curiosity why take it by value and not rvalue_reference?

The lvalue_reference case is already covered by the classical std::begin() from <iterator>. So what do you gain by not writing basic_string_view&& _Right?

As I understand, it's just a performance optimization. In case of (const-)reference you'll do dereference twice while accessing to the underlying c-string's character - the first dereference of 'reference' itself (because references are implemented as pointers), the second is dereference of the c-string. If you pass string_view by value, one avoids the first dereference.
And, of course, it's worth to mention that string_view is a small object so it's cheep to copy them. Dereference could take much more time because it involves accessing to memory which is slow comparing to CPU, especially in case of cache-missing.

[miscco]

@shuffle-c That overload is not about performance but to provide a specialization for begin(T&&) which is otherwise declared as deleted ("poison pill").

That specialization is required to opt-in the forwarding-range concept.

[CaseyCarter]

Out of curiosity why take it by value and not rvalue_reference?

These non-member begin and end overloads need to accept both const and non-const rvalues. We could correctly declare begin(basic_string_view&&) and begin(const basic_string_view&&) (and likewise two end overloads) but it's simpler to simply declare one begin and one end that take a basic_string_view by value and can therefore accept all permutations of const-ness and value category.