From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13119 invoked by alias); 4 Dec 2018 14:38:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 13095 invoked by uid 89); 4 Dec 2018 14:38:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=_Hashtable, _hashtable, Dumont, franois X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Dec 2018 14:38:38 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 84F293084218; Tue, 4 Dec 2018 14:38:37 +0000 (UTC) Received: from localhost (unknown [10.33.36.140]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3436D5C6BE; Tue, 4 Dec 2018 14:38:37 +0000 (UTC) Date: Tue, 04 Dec 2018 14:38:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: Fix move_if_noexcept usages in _Hashtable Message-ID: <20181204143836.GC27131@redhat.com> References: <696186d1-b00c-4c42-6228-f00bba8e40ff@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <696186d1-b00c-4c42-6228-f00bba8e40ff@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-SW-Source: 2018-12/txt/msg00195.txt.bz2 On 04/12/18 07:45 +0100, François Dumont wrote: >Hi > >  This patch fix a minor problem with usage of std::move_if_noexcept. >We use it to move node content if move construtor is noexcept but we >eventually use the allocator_type::construct method which might be >slightly different. I think it is better to check for this method >noexcept qualification. This is likely to pessimize some code, since most allocators do not have an exception-specification on their construct members. >  Moreover I have added a special overload for nodes containing a >std::pair. It is supposed to allow move semantic in associative >containers where Key is stored as const deleting std::pair move >constructor. In this case we should still move the Value part. > >  It doesn't work for the moment because the std::pair piecewise >constructor has no noexcept qualification. Is there any plan to add it >? I think adding it will force including in stl_pair.h, is it >fine ? > >  If this evolution is accepted I'll adapt it for _Rb_tree that has >the same problem. > >  Working on this I also notice that content of initialization_list is >not moved. Is there a plan to make initialization_list iterator type >like move_iterator ? Should containers use >__make_move_iterator_if_noexcept ? No. Whether to allow moving from std::initializer_list is an active topic, see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1249r0.html >  Tested under Linux x86_64 normal mode. > >  Ok to commit this first step ? No, this is not suitable for stage 3. It seems too risky. We can reconsider it during stage 1, but I'd like to see (at least) a new test showing a bug with the current code. For example, a type with a move constructor that is noexcept, but when used with a scoped_allocator_adaptor (which calls something other than the move constructor) we incorrectly move elements, and lose data when an exception happens.