From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by sourceware.org (Postfix) with ESMTPS id 57F5A384F02A; Fri, 21 May 2021 06:48:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 57F5A384F02A Received: by mail-wm1-x32e.google.com with SMTP id z137-20020a1c7e8f0000b02901774f2a7dc4so6262457wmc.0; Thu, 20 May 2021 23:48:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MbwIPtqQYl7+jm7fN4hgyZjD6aQlP98ancVcmjM1mJ8=; b=PJ+U+LJFSsPvyPUCyHOEl3pwhUpxH+saE68Um0V5Bg7Y92KfChai7O+Is78VEnOTd4 jFaySWT3wFhb6uMVwDuJwlXyGCTjFo7lDRJD3gLKGESxS3FbbXKVUtpbVACtyLP7+eGQ EElwKk2EsZL22Km11cC1O70wl7DnqdH4Jv/2rN9190FPGxUpd7/TG9tJLJl9NUApt15J feoXV2dwaq70q2rWK12/6YQKGpc9G4IUWNel4QGKVYeSprUiY6ZIiW5hyEnVAYLpAdIG 7KSCEgV6GZnnMX+Kgk0VHNzknlB9pLMsewsSpq7/u4H6uMA4Aqgi7inN73ewM6yQWPra bVUw== X-Gm-Message-State: AOAM533RduVuf6IbGUUhBZhtclfrozitHNAojidX7ut6PzoIBZYTj9SD rZHrdK+cvUhXJgzxnsoWUR/CPu8ESwIBmjt2arE= X-Google-Smtp-Source: ABdhPJyfg7H341JQxXWa61xf8YqDUfnly2nRySx1SBAHpu0K/+nMVqBGwaSsLckUL9YrAV7TnZZAWqS/i94TxKmg888= X-Received: by 2002:a05:600c:2310:: with SMTP id 16mr7180691wmo.17.1621579725305; Thu, 20 May 2021 23:48:45 -0700 (PDT) MIME-Version: 1.0 References: <5b198dd5-cb89-91a0-9070-13927ac263a4@gmail.com> <524e2eee-a4ee-a05e-087f-6000c8274eff@gmail.com> <21854fd0-ad6b-1eaa-adaa-2074421fc107@gmail.com> <7bd748f6-77bd-fdcc-f925-1700ac9da3de@gmail.com> <50b5f8f5-56a3-57a5-2e03-b23118a1a2c5@gmail.com> In-Reply-To: <50b5f8f5-56a3-57a5-2e03-b23118a1a2c5@gmail.com> From: Jonathan Wakely Date: Fri, 21 May 2021 07:48:33 +0100 Message-ID: Subject: Re: [PATCH] Hashtable PR96088 To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: Jonathan Wakely , "libstdc++" , gcc-patches X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 May 2021 06:48:51 -0000 On Fri, 21 May 2021, 07:31 Fran=C3=A7ois Dumont via Libstdc++, < libstdc++@gcc.gnu.org> wrote: > On 20/05/21 6:44 pm, Jonathan Wakely wrote: > > On 06/05/21 22:03 +0200, Fran=C3=A7ois 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 can be specialized by users, > > and is not required to provide argument_type. > > > > So it's already not valid to assume that std::hash::argument_type > > exists. > > Yes, I know that the plan is to get rid of argument_type. But as long as > it is there we can still use it even if I didn't realize that you were > already in the process of removing it so soon. > I'm adding a new specialization for a C++23 type, and not adding the nested types. > > > > >> @@ -850,9 +852,56 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > >> iterator > >> _M_emplace(const_iterator, false_type __uks, _Args&&... __args); > >> > >> + template > >> + std::pair > >> + _M_insert_unique(_Kt&&, _Arg&&, const _NodeGenerator&); > >> + > >> + // Detect nested argument_type. > >> + template> > >> + struct _Hash_arg_t > >> + { typedef _Kt argument_type; }; > >> + > >> + // std::hash > >> + template > >> + struct _Hash_arg_t<_Kt, std::hash<_Arg>> > >> + { typedef _Arg argument_type; }; > >> + > >> + // Nested argument_type. > >> + template > >> + struct _Hash_arg_t<_Kt, _Ht, > >> + __void_t> > >> + { typedef typename _Ht::argument_type argument_type; }; > >> + > >> + // Function pointer. > >> + template > >> + struct _Hash_arg_t<_Kt, std::size_t(*)(const _Arg&)> > >> + { typedef _Arg argument_type; }; > >> + > >> + template >> + typename _ArgType > >> + =3D 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? > > No, the purpose of _Hash_arg_t is to find out what is the argument type > of the _Hash functor. With this info I can check if invoking that > functor is going to instantiate a temporary using a throwing operation. Right, that's what I mean. > If so, I create a temporary at _Hashtable code level and move it to its > final storage place when needed. > > The tricky part is that _Hash can accept different argument types, for > the moment I just do not create a temporary in this case. > > > > > 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? > > > I think I already try but I'll check. > > I fear that __is_nothrow_invocable<_Hash&, _Kt> tells if the chosen > operator()(const _Arg&) is noexcept qualified. No. Not if the conversion > from _Kt to _Arg is noexcept. > It does exactly what you need. > >