Re: [PATCH] Hashtable PR96088

[Jonathan Wakely <jwakely@redhat.com>]

On 06/05/21 22:03 +0200, François Dumont via Libstdc++ wrote:
>Hi
>
>    Considering your feedback on backtrace in debug mode is going to 
>take me some time so here is another one.
>
>    Compared to latest submission I've added a _Hash_arg_t partial 
>specialization for std::hash<>. It is not strictly necessary for the 
>moment but when we will eventually remove its nested argument_type it 
>will be. I also wonder if it is not easier to handle for the compiler, 
>not sure about that thought.

The std::hash specializations in libstdc++ define argument_type, but
I'm already working on one that doesn't (forstd::stacktrace).

And std::hash<acme::ProgramDefinedType> can be specialized by users,
and is not required to provide argument_type.

So it's already not valid to assume that std::hash<T>::argument_type
exists.

>@@ -850,9 +852,56 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	iterator
> 	_M_emplace(const_iterator, false_type __uks, _Args&&... __args);
> 
>+      template<typename _Kt, typename _Arg, typename _NodeGenerator>
>+	std::pair<iterator, bool>
>+	_M_insert_unique(_Kt&&, _Arg&&, const _NodeGenerator&);
>+
>+      // Detect nested argument_type.
>+      template<typename _Kt, typename _Ht, typename = __void_t<>>
>+	struct _Hash_arg_t
>+	{ typedef _Kt argument_type; };
>+
>+      // std::hash
>+      template<typename _Kt, typename _Arg>
>+	struct _Hash_arg_t<_Kt, std::hash<_Arg>>
>+	{ typedef _Arg argument_type; };
>+
>+      // Nested argument_type.
>+      template<typename _Kt, typename _Ht>
>+	struct _Hash_arg_t<_Kt, _Ht,
>+			  __void_t<typename _Ht::argument_type>>
>+	{ typedef typename _Ht::argument_type argument_type; };
>+
>+      // Function pointer.
>+      template<typename _Kt, typename _Arg>
>+	struct _Hash_arg_t<_Kt, std::size_t(*)(const _Arg&)>
>+	{ typedef _Arg argument_type; };
>+
>+      template<typename _Kt,
>+	       typename _ArgType
>+		 = typename _Hash_arg_t<_Kt, _Hash>::argument_type>
>+	static typename conditional<
>+	  __is_nothrow_convertible<_Kt, _ArgType>::value, _Kt&&, key_type>::type

Please use __conditional_t<...> here instead of
typename conditional<...>::type.

The purpose of the _Hash_arg_t type is to determine whether invoking
the hash function with _Kt&& can throw, right?

And if it can throw, you force a conversion early, and if it can't,
you don't do the conversion.

Can't you use __is_nothrow_invocable<_Hash&, _Kt> for that, instead of
this fragile approach?