From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70786 invoked by alias); 4 Dec 2018 21:43:29 -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 70754 invoked by uid 89); 4 Dec 2018 21:43:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=sk:www.ope, _Hashtable, _hashtable, fran=c3=a7ois?= X-HELO: mail-wr1-f54.google.com Received: from mail-wr1-f54.google.com (HELO mail-wr1-f54.google.com) (209.85.221.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Dec 2018 21:43:26 +0000 Received: by mail-wr1-f54.google.com with SMTP id c14so17550484wrr.0; Tue, 04 Dec 2018 13:43:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=2qqY6tBYzHcV2tZe+xUP9gxpSEf1EmRFbMnLX+FOtBE=; b=Z53HO/nwlolTFWMDKIn0fhtSBoE0N+5KInYXoojBo+NX++x8WiDKX/aawOBoaLFdle UJmT5Dhm8uOLVPKVY9jYoDU4gPCOb+niu8L+rbih4D/t1kmLs723cNAOLM4S/ocMO9zo wJdy0t1korufXe6F/AznjR0q7Qw7bQ4E9M3kwnWTXmBqczxHxawlZdUzkjWdjw799s/r 19UwlU2XB1ZSYftp7PG+hyGIiigg1QQxBCKe7nsWcG2vbsiNrwIr9voI+Bal4uhidzZS qjipxk1HJ6amOfAKOyJwYZPusb1lMz47OVGJV6Egn3j8tI3iZds8ChIXpWzDs05fz7no yBcA== Return-Path: Received: from [192.168.0.22] ([91.167.114.199]) by smtp.googlemail.com with ESMTPSA id t18sm5577686wmt.35.2018.12.04.13.43.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 04 Dec 2018 13:43:22 -0800 (PST) Subject: Re: Fix move_if_noexcept usages in _Hashtable To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <696186d1-b00c-4c42-6228-f00bba8e40ff@gmail.com> <20181204143836.GC27131@redhat.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Message-ID: <81e3fa4e-e4cc-67b5-0329-e6800cc1d62a@gmail.com> Date: Tue, 04 Dec 2018 21:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181204143836.GC27131@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2018-12/txt/msg00235.txt.bz2 On 12/4/18 3:38 PM, Jonathan Wakely wrote: > 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. > Perhaps but the Standard mandates to call allocator construct so we don't have choice. It is surprising to consider value_type move constructor when we don't know what allocator construct does. Most users do not change from std::allocator or, even if they do, do not implement construct so impact should be limited. >>   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 ? No feedback on this point ? Is using std::pair piecewise constructor ok ? >> >>   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 > Ok, nice, allowing emplace build of values would be even better, I'll have a closer look. >>   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. > > Ok, I'll try.