<xstring>: Too many enabled hash specializations

[CaseyCarter]

Per the standard, the specializations:

template<> struct hash<string>;
template<> struct hash<u8string>;
template<> struct hash<u16string>;
template<> struct hash<u32string>;
template<> struct hash<wstring>;
template<> struct hash<pmr::string>;
template<> struct hash<pmr::u8string>;
template<> struct hash<pmr::u16string>;
template<> struct hash<pmr::u32string>;
template<> struct hash<pmr::wstring>;

template<> struct hash<string_view>;
template<> struct hash<u8string_view>;
template<> struct hash<u16string_view>;
template<> struct hash<u32string_view>;
template<> struct hash<wstring_view>;

Should be enabled, and all other specializations of hash for specializations of basic_string_view / basic_string should be disabled. We currently enable hash for all specializations of basic_string_view:

STL/stl/inc/xstring

Lines 1715 to 1724 in 852a308

	// STRUCT TEMPLATE SPECIALIZATION hash FOR basic_string_view
	template <class _Elem, class _Traits>
	struct hash<basic_string_view<_Elem, _Traits>> {
	_CXX17_DEPRECATE_ADAPTOR_TYPEDEFS typedef basic_string_view<_Elem, _Traits> _ARGUMENT_TYPE_NAME;
	_CXX17_DEPRECATE_ADAPTOR_TYPEDEFS typedef size_t _RESULT_TYPE_NAME;
	_NODISCARD size_t operator()(const basic_string_view<_Elem, _Traits> _Keyval) const noexcept {
	return _Hash_array_representation(_Keyval.data(), _Keyval.size());
	}
	};

and basic_string:

STL/stl/inc/xstring

Lines 4579 to 4588 in 852a308

	// STRUCT TEMPLATE SPECIALIZATION hash
	template <class _Elem, class _Traits, class _Alloc>
	struct hash<basic_string<_Elem, _Traits, _Alloc>> {
	_CXX17_DEPRECATE_ADAPTOR_TYPEDEFS typedef basic_string<_Elem, _Traits, _Alloc> _ARGUMENT_TYPE_NAME;
	_CXX17_DEPRECATE_ADAPTOR_TYPEDEFS typedef size_t _RESULT_TYPE_NAME;
	_NODISCARD size_t operator()(const basic_string<_Elem, _Traits, _Alloc>& _Keyval) const noexcept {
	return _Hash_array_representation(_Keyval.c_str(), _Keyval.size());
	}
	};

Additional Context
Skipped libcxx test

STL/tests/libcxx/expected_results.txt

Lines 640 to 642 in 06827fe

	# STL bug: GH-757 <xstring>: Too many enabled hash specializations
	std/strings/basic.string.hash/char_type_hash.fail.cpp FAIL
	std/strings/string.view/string.view.hash/char_type.hash.fail.cpp FAIL

[CaseyCarter]

Per the standard, the specializations:

This notably excludes basic_string specializations with allocator types other than std::allocator even if they have reasonable character and traits types. basic_string<char, char_traits<char>, my_allocator<char>>, for example, should be hashable despite the nonstandard allocator: the character encoding has nothing to do with the allocator. I'll poke LWG about this.

[CaseyCarter]

I'll poke LWG about this.

I don't know if this ever happened, but I did just submit an LWG issue to change the explicit specializations of hash for basic_string to partial specializations that don't depend on allocator type.

[CaseyCarter]

I'm marking this issue as needing a decision. If we apply the change this issue is asking for, we may break code in the wild that uses std::hash for basic_string specializations with custom allocators. Do we want to wait for LWG to deal with the submitted-but-not-yet-numbered issue that will allow hashing strings with arbitrary allocators? Do we want to speculatively implement the resolution? Do we want to make this change now and worry about the LWG issue later?

Pinging @frederick-vs-ja whose open PR is impacted by the outcome of this decision - feel free to comment.

[StephanTLavavej]

I'm OK with making this change now (at the beginning of the 17.4 cycle) and then implementing the LWG issue once it's accepted (or at least Tentatively Ready). There's no ABI or separately compiled impact, so we aren't locking ourselves into a decision, and if it's a widely breaking change, we should hear about it early (including from RWC coverage). What the Standard currently says is clear and internally consistent, so our usual rule is to do what the Standard says, even if it might be changed to be less restrictive in the future. (Vaguely similar precedent would be how uniform_int_distribution<unsigned char> is forbidden.)

[frederick-vs-ja]

At the first time cppreference mentioned LWG-2978, I was surprised to find that the resolution was not generalized to all possible allocators. Providing specializations for pmr types strongly implies that the difference of memory resources should be ignored when hashing, so I have no idea why the resolution was restricted to strings with polymorphic_allocator.

LWG-2978 also mentioned an alternative approch, but it was not adopted.

An alternative approach, for which wording can be produced if desired, is to make the hash specializations for basic_string allocator-agnostic, similar to the partial specialization of hash for vector<bool>.

Asking @timsong-cpp why wasn't the alternative approch proposed.

[CaseyCarter]

Asking @timsong-cpp why wasn't the alternative approach proposed.

From LWG's internal discussion on the Toronto wiki, LWGChair was of the opinion that doing so would be a design change beyond the mandate of "making sure all the pmr types are hashable" that they were working on at the time. I would have argued differently had I been in the room (think I was in LEWG working on Ranges NB comments).

In any case, the current LWG chair seems to agree with me that the change is a good consistency fix (after mocking me for the issue title "hashability shouldn't depend on basic_string_view's allocator", of course 🤦‍♂️).