Adapt some return statements to P2266R1 "Simpler implicit move"

[stbergmann]

...as implemented by Clang 13 trunk in C++23 mode (see
https://reviews.llvm.org/D99005 "[clang] Implement P2266 Simpler implicit
move"). These are the two issues I came across when building LibreOffice with
clang-cl; there may be more such issues across STL:

In file included from C:/lo-clang/core/setup_native/source/win32/customactions/reg_dlls/reg_dlls.cxx:12:
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\string(66,12): error: non-const lvalue reference to type 'basic_istream<...>' cannot bind to a temporary of type 'basic_istream<...>'
    return _Istr;
           ^~~~~
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\string(78,12): note: in instantiation of function template specialization 'std::getline<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t>>' requested here
    return getline(_STD move(_Istr), _Str, _Delim);
           ^
C:/lo-clang/core/setup_native/source/win32/customactions/reg_dlls/reg_dlls.cxx(197,17): note: in instantiation of function template specialization 'std::getline<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t>>' requested here
    while (std::getline(ss, sToken, L'|'))
                ^

and

In file included from C:/lo-clang/core/l10ntools/source/merge.cxx:21:
In file included from C:/lo-clang/core/include\sal/log.hxx:20:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\sstream:11:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\istream:11:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\ostream:11:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\ios:11:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xlocnum:16:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\streambuf:11:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xiosbase:12:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\system_error:14:
In file included from C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\stdexcept:12:
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xstring(4971,12): error: non-const lvalue reference to type 'basic_istream<...>' cannot bind to a temporary of type 'basic_istream<...>'
    return _Istr;
           ^~~~~
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xstring(4977,29): note: in instantiation of function template specialization 'std::operator>><char, std::char_traits<char>, std::allocator<char>>' requested here
    return _STD move(_Istr) >> _Str;
                            ^
C:/lo-clang/core/l10ntools/source/merge.cxx(125,18): note: in instantiation of function template specialization 'std::operator>><char, std::char_traits<char>, std::allocator<char>>' requested here
    aInputStream >> sPoFile;
                 ^

[ghost]

All CLA requirements met.

[cpplearner]

The affected overload

template <class _Elem, class _Traits, class _Alloc>
basic_istream<_Elem, _Traits>& operator>>(
    basic_istream<_Elem, _Traits>&& _Istr, basic_string<_Elem, _Traits, _Alloc>& _Str) {

(which takes an rvalue reference to basic_istream) is not depicted in the standard. Could we remove this overload and rely on

STL/stl/inc/istream

Lines 886 to 891 in 1866b84

	template <class _Istr, class _Ty,
	enable_if_t<conjunction_v<is_convertible<_Istr*, ios_base*>, _Can_stream_in<_Istr, _Ty>>, int> = 0>
	_Istr&& operator>>(_Istr&& _Is, _Ty&& _Val) { // extract from rvalue stream
	_Is >> _STD forward<_Ty>(_Val);
	return _STD move(_Is);
	}

to handle rvalue istream?

[StephanTLavavej]

Thanks for the PR! Please let us know if you have any issues with the CLA.

By the way, GitHub is displaying the error message logs with strikethroughs because of the embedded ~ characters in the short filename components. Instead of quoting the error messages with > at the beginning of each line, putting them in a code block with triple backticks will both avoid the strikethroughs, and will display the messages in monospace so Clang's caret diagnostics will be readable.

[mizvekov]

FYI, we are implementing workarounds in clang for this issue.
We recently committed a change which disables P2266 when compiling with "-fms-compatibility" as an interim solution, to save having to revert the whole thing.

The current idea (but this is still under discussion) is to ship clang-13 with a workaround where we somehow detect we are dealing with the MSVC STL and disable P2266 only for code within it.
But any feedback is welcome!

I appreciate the workaround might make things harder for you to test it, since this is tied to that one flag and you might have other issues that prevent you from not using it.

[StephanTLavavej]

Thanks @mizvekov - we test the STL with -fno-ms-compatibility for maximum strictness, so I don't think that your workaround will interfere. I think I'd prefer for Clang to not special-case MSVC's STL, and instead have -fms-compatibility be the escape hatch for older STL versions. Right now, STL changes will flow into VS 2022 17.1, but we can backport changes to VS 2022 17.0 with a bit of additional effort if this is expected to be a headache.

[mizvekov]

I see, thanks for the suggestion @StephanTLavavej !

I could totally live with just making this be dependent on certain ms-compatibility-version.
We could leave the decision of placing a version check there when an actual cl.exe which has P2266 is shipped.

Though the main objective of implementing this feature so early was to get implementation experience that the committee asked for. And this would basically exclude getting feedback from everyone using those older versions.
So that's why one of the ideas is to make this be dependent on -fms-compatibility(-version when available) + only applies to actual STL system headers (so we would exclude libc++ from the workaround).

[StephanTLavavej]

Thanks for fixing these compiler errors, and congratulations on your first microsoft/STL commit! 🎉 😸 🚀

This will ship in VS 2022 17.0 Preview 4.