<xstring>: Strengthen exception specification for sv.compare("NTBS")

[frederick-vs-ja]

As discovered in #3735, both

• basic_string_view's constructor from a const _Elem* (strengthened) and
• the basic_string_view::compare overload for two basic_string_views (mandatory)

are already noexcept, so basic_string_view::compare for a basic_string_view and a const _Elem* should also be noexcept.

The exception specification strengthening is already done for basic_string.

STL/stl/inc/xstring

Lines 4646 to 4649 in a621095

	_NODISCARD _CONSTEXPR20 int compare(_In_z_ const _Elem* const _Ptr) const noexcept /* strengthened */ {
	// compare [0, size()) with [_Ptr, <null>)
	return _Traits_compare<_Traits>(_Mypair._Myval2._Myptr(), _Mypair._Myval2._Mysize, _Ptr, _Traits::length(_Ptr));
	}

The message in the static_assert "(this is bad, char_traits)" no longer holds, since the exception specifications of all char_traits::compare in MSVC STL are already strengthened.

---

Driven-by changes: make basic_string::_Eos, basic_string::_Swap_proxy_and_iterators, and basic_string::_Swap_data noexcept since they are non-throwing and already called by noexcept functions.

[achabense]

More candidates:
_Move_assign_from_buffer and _Copy_assign_val_from_small:
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L2908?plain=1
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L3131?plain=1
_Become_small then reserve(void):
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L4796?plain=1
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L4191?plain=1
_Eos:
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L4813?plain=1
_Swap_proxy_and_iterators and _Swap_data:
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L4853?plain=1
https://github.com/microsoft/STL/blob/a62109595b6d89e08172fdf4beb75a2670fe0cc9/stl/inc/xstring#L4261?plain=1

[frederick-vs-ja]

I think we should do strengthening for _Eos, _Swap_proxy_and_iterators, and _Swap_data since they are already called by noexcept functions.

I'm hesitant about other functions, because

• _Move_assign_from_buffer and _Copy_assign_val_from_small are only called from functions that may throw an exception,
• reserve(void) is deprecated, and other invocations of _Become_small are only from functions that may throw an exception (like the above).

[achabense]

_Move_assign_from_buffer:
It doesn't involve memory allocation or other throwable operations.
The caller sstream::str()&& (here) is throwable due to assignment in another place.

_Copy_assign_val_from_small:
It doesn't involve memory allocation or other throwable operations.
The caller operator=(const basic_string&) (here) is throwable due to assignment in another place.
(off topic) I saw // TRANSITION, VSO-761321; inline into only caller when that's fixed in its code. What is it? Can it be fixed now?

_Become_small:
It's used to deallocate superfluous memory. Compared with _Tidy_deallocate (here) I believe it's safe to add noexcept.
The callers are reserves. They are throwable due to some potential re-allocations. As to reserve(void), it doesn't involve such kind of re-allocation. Though deprecated, a noexcept /*strengthened*/ is very cheap to add so I think it's ok.

(off topic)
basic_string(nullptr_t) is now inserted between basic_string(const _Elem*) and basic_string(const _Elem*, const _Alloc& ). The latter two work as a whole to resolve as basic_string(const _Elem*, const _Alloc& = _Alloc()). For better maintainability, please consider moving basic_string(nullptr_t) elsewhere.

STL/stl/inc/xstring

Lines 2539 to 2553 in a621095

	_CONSTEXPR20 basic_string(_In_z_ const _Elem* const _Ptr) : _Mypair(_Zero_then_variadic_args_t{}) {
	_Construct<_Construct_strategy::_From_ptr>(_Ptr, _Convert_size<size_type>(_Traits::length(_Ptr)));
	}
	#if _HAS_CXX23
	basic_string(nullptr_t) = delete;
	#endif // _HAS_CXX23
	#if _HAS_CXX17
	template <class _Alloc2 = _Alloc, enable_if_t<_Is_allocator<_Alloc2>::value, int> = 0>
	#endif // _HAS_CXX17
	_CONSTEXPR20 basic_string(_In_z_ const _Elem* const _Ptr, const _Alloc& _Al)
	: _Mypair(_One_then_variadic_args_t{}, _Al) {
	_Construct<_Construct_strategy::_From_ptr>(_Ptr, _Convert_size<size_type>(_Traits::length(_Ptr)));
	}

[StephanTLavavej]

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[strega-nil-ms]

nit

[strega-nil-ms]

@StephanTLavavej minor change pushed

[StephanTLavavej]

@strega-nil-ms I believe you have flipped the sense of the message; compare the previous message.

(The message is admittedly confusing about whether it's stating what is currently observed, or what should be observed.)

[StephanTLavavej]

@strega-nil-ms @CaseyCarter I've pushed changes to restore the // strengthened comment, then mechanically replace the entire test to use terse static_assert as it was already doing in several places (yay for C++17). For the test suite, static_assert messages are much less useful than in product code, so I didn't restrict this replacement to just the noexcept stuff, I did the whole file.

[StephanTLavavej]

🦾 ⚠️ 🧶