From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 585E03858D34; Thu, 2 Jul 2020 11:38:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 585E03858D34 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1593689910; bh=Yx1fQ0pKTa4soiz2aJNUthctr0JrDhvnpN1s1H48w7A=; h=From:To:Subject:Date:From; b=s/nQjZ3sUMg6TE8H6utpYszlA4xVUciDAswgLiz8qUpSew44/+uZklfd9sWU1oIKy yr7mwFtYspGRAW4gEEOOmdn9UeZxmh++ja+Tt8sT7tMXciHs5ndQVWV3pKbBoiXWeU pxxTbgpAV4772DBocGaDiKtYGsKp36jYjZeVb9RM= From: "nknikita at niisi dot ras.ru" To: gcc-bugs@gcc.gnu.org Subject: [Bug libstdc++/96029] New: Inconsistencies with associative/unordered containers Date: Thu, 02 Jul 2020 11:38:30 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: libstdc++ X-Bugzilla-Version: unknown X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: nknikita at niisi dot ras.ru X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter target_milestone Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jul 2020 11:38:30 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D96029 Bug ID: 96029 Summary: Inconsistencies with associative/unordered containers Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: libstdc++ Assignee: unassigned at gcc dot gnu.org Reporter: nknikita at niisi dot ras.ru Target Milestone: --- Well, it seems that there some problems with noexcept specification of associative and unordered containers' move constructors and move assignment operators. The following is the reply that I have received from Jonathan Wa= kely via GCC's mailing list. ---------------------------------------------------------------------------= --- On 01/07/20 19:10 +0300, =D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0 =D0=91=D0=B0= =D0=B9=D0=BA=D0=BE=D0=B2 wrote: > Hello, > > I'm trying to make sense of associative and unordered containers' behavio= r. Some of the implementation details seem a little inconsistent to me. Cou= ld you please take a look? > > Most of my questions concern move constructors and assignments, and their= noexcept specifications. To be more specific, let me list some examples. > > 1. Move constructor for unordered_set is explicitly defaulted on its firs= t declaration: > > /// Move constructor. > unordered_set(unordered_set&&) =3D default; > > Since unordered_set has no base classes and has just one data member of t= ype _Hashtable, its noexcept specification would be derived from the follow= ing: > > template typename _Alloc, typename _ExtractKey, typename _Equal, > typename _H1, typename _H2, typename _Hash, typename _RehashPolicy, > typename _Traits> > _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, > _H1, _H2, _Hash, _RehashPolicy, _Traits>:: > _Hashtable(_Hashtable&& __ht) noexcept > : __hashtable_base(__ht), > /* ... the of the initializer list and the constructor body */ > > This move constructor is marked noexcept unconditionally despite the fact= it has to copy hash function _H1 and function _Equal that compares the key= s. Looks like a bug. It was introduced by https://gcc.gnu.org/g:0462b6aa20fd6734f7497f5eed9496d33701a952 This program exited normally with GCC 4.8 and terminates since 4.9.0: #include bool boom =3D false; struct Eq : std::equal_to { Eq() { } Eq(const Eq&) { if (boom) throw 1; } }; int main() { std::unordered_set, Eq> s; try { boom =3D true; auto s2 =3D std::move(s); } catch (int) { } } > At the same time, its move assignment operator is also explicitly default= ed: > > /// Move assignment operator > unordered_set& > operator=3D(unordered_set&&) =3D default; > > and therefore is conditionally noexcept, since the corresponding _Hashtab= le::operator=3D is declared as > > _Hashtable& > operator=3D(_Hashtable&& __ht) > noexcept(__node_alloc_traits::_S_nothrow_move() > && is_nothrow_move_assignable<_H1>::value > && is_nothrow_move_assignable<_Equal>::value); > > So, the first question is why the copy assignment of _H1 and _Equal is co= nsidered to never throw, while the move assignment is not. As a side=20 Looks like a bug. > question, why does the move constructor choose to copy _H1 and _Equal ins= tead of moving them? Howard Hinnant asked me about that recently, because we do the same for maps and sets as well as for unordered maps and sets. It's probably wrong, but I don't really want to change it until https://cplusplus.github.io/LWG/issue2227 is resolved. > 2. Let's assume that there is nothing wrong with noexcept specifications = in unordered containers, i.e., all constructors and assignment operators ar= e marked noexcept as they ought to be. Why then are the associative contain= ers not implemented the same way? As far as I can see in bits/stl_set.h and= bits/stl_tree.h, > > // Move constructor > set(set&&) =3D default; > > // The only data member of set is _Rb_tree object, whose move construc= tor is explicitly defaulted > _Rb_tree(_Rb_tree&&) =3D default; > > // The only data member of _Rb_tree is _Rb_tree_impl<_Compare> object = that is defined as > template bool /* _Is_pod_comparator */ =3D __is_pod(_Key_compare)> > struct _Rb_tree_impl > : public _Node_allocator > , public _Rb_tree_key_compare<_Key_compare> > , public _Rb_tree_header > { > _Rb_tree_impl() =3D default; > _Rb_tree_impl(_Rb_tree_impl&&) =3D default; > > /* the rest of the definition */ > }; > > // Finally, _Rb_tree_key_compare move constructor is defined as > _Rb_tree_key_compare(_Rb_tree_key_compare&& __x) > noexcept(is_nothrow_copy_constructible<_Key_compare>::value) > : _M_key_compare(__x._M_key_compare) > { } > > What is the difference between _Compare (for associative containers) and = _Equal (for unordered containers) in regard to noexcept? Why we=20 One is correct and the other isn't. > have to check is_nothrow_copy_constructible value in the case of _Compare= but can ignore it in the case of _Equal? > > > 3. The move constructor declaration and the allocator-extended move const= ructor declaration for std::set result into different noexcept specificatio= ns. Isn't that a small defect of the class interface? > > /// Allocator-extended move constructor. > set(set&& __x, const allocator_type& __a) > noexcept(is_nothrow_copy_constructible<_Compare>::value > && _Alloc_traits::_S_always_equal()) > : _M_t(std::move(__x._M_t), _Key_alloc_type(__a)) { } > > /// Move constructor > set(set&&) =3D default; > > Given the declarations above, I reckon that noexcept specification of the= explicitly defaulted constructor can be rewritten as > > is_nothrow_copy_constructible<_Compare>::value && is_nothrow_move_cons= tructible::value Allocators aren't allowed to throw when copied or moved, so the is_nothrow_move_constructible check is not needed. The fact it's implied by the defaulted constructor does seem like a bug. > Let's say someone defined a custom empty allocator type but didn't mark i= ts move constructor as noexcept (besides the fact that move constructors sh= all never throw). noexcept conditions evaluation yields different results. They should fix their allocator. Explicitly providing a move constructor for an allocator is silly anyway. Don't do that. If you insist on it, get the exception specification right. > P.S. Sorry if this is an inappropriate place to ask these questions. Sinc= e I am not truly convinced that any of these is a bug (more likely a kind o= f GCC's extension to the standard), I decided to not post them on the bug t= racker. Hope that you would suggest the proper place then. Please do report it to bugzilla, because the assertion in the code below used to pass, but fails since GCC 7.1.0, due to the changes in https://gcc.gnu.org/g:a4dec0d6de97348e71932f7080fe4a3bb8730096 #include template struct Alloc { using value_type =3D T; Alloc() { } Alloc(const Alloc&) { } template Alloc(const Alloc&) { } T* allocate(std::size_t n) { return std::allocator().allocate(n); } void deallocate(T* p, std::size_t n) { std::allocator().deallocate(p, n); } }; static_assert( std::is_nothrow_move_constructible, Alloc>>::value, "" ); Fran=C3=A7ois, could you please take a look?=