From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id D748D3857B9C; Tue, 24 May 2022 20:25:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D748D3857B9C Received: by mail-wm1-x336.google.com with SMTP id bg25so11258855wmb.4; Tue, 24 May 2022 13:25:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=cNGt0Uo70nRz4RIlgBl3cjy0f03Gpb3OcPreDJQUA0o=; b=H5JBcaqtGLNVRsEvok1Odgfm6sTGrG+nnTuDYuys24UIxdawcpxCjLda9brPzue0ox 3rLkooLx3Y0fq2g6EUbnsSVdbmSIcjkbRDme/x4l+9D+Nxax/PyqkBCR0ZvUwjMJMWf/ yBjRFNbpjlx8mAq44yXboTd7MU3kFIZbxE8UCEgThayjWOLcwtQmxy/Knr5gqEc3eTNs c+rorpHjorUr4FyqEnGkIoFvl3eKixq1I/TdpW9qS/9eZzhAFJpm5Ysq7ucVrlEK0Y5t Qg50PRZm/9Bd78fSlxYGA55bbWXd2gnQ2Feb2tMdr32KKNBJf6iFTp8I9BP8XjNiBnOy YNwA== X-Gm-Message-State: AOAM533o+6/PCJcMR1D4fAziO787lguU1hKOi+X26uv9rFsdirn3Hlvr 9HnsOHZO4BfdFUhnTab3PWw= X-Google-Smtp-Source: ABdhPJxfyt5xasUN5yMIz02LkjjqSrnHoN6d5iV5/SbfQNECFmV/AL9BIYZnj9kuEr3Zm66pDPLVNQ== X-Received: by 2002:a05:600c:a08:b0:392:a561:9542 with SMTP id z8-20020a05600c0a0800b00392a5619542mr5253766wmp.62.1653423911335; Tue, 24 May 2022 13:25:11 -0700 (PDT) Received: from ?IPV6:2a01:e0a:1dc:b1c0:38cf:2698:fecd:33db? ([2a01:e0a:1dc:b1c0:38cf:2698:fecd:33db]) by smtp.googlemail.com with ESMTPSA id u14-20020a5d434e000000b0020c5253d8fcsm378207wrr.72.2022.05.24.13.25.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 May 2022 13:25:10 -0700 (PDT) Message-ID: Date: Tue, 24 May 2022 22:25:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH][_Hashtable] Fix insertion of range of type convertible to value_type PR 56112 Content-Language: fr To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <2fe3937d-1b9a-a547-bb41-225d3d5426a2@gmail.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: Tue, 24 May 2022 20:25:18 -0000 On 24/05/22 12:18, Jonathan Wakely wrote: > On Thu, 5 May 2022 at 18:38, François Dumont via Libstdc++ > wrote: >> Hi >> >> Renewing my patch to fix PR 56112 but for the insert methods, I totally >> change it, now works also with move-only key types. >> >> I let you Jonathan find a better name than _ValueTypeEnforcer as usual :-) >> >> libstdc++: [_Hashtable] Insert range of types convertible to value_type >> PR 56112 >> >> Fix insertion of range of types convertible to value_type. Fix also when >> this value_type >> has a move-only key_type which also allow converted values to be moved. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/56112 >> * include/bits/hashtable_policy.h (_ValueTypeEnforcer): New. >> * include/bits/hashtable.h >> (_Hashtable<>::_M_insert_unique_aux): New. >> (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, >> true_type)): Use latters. >> (_Hashtable<>::_M_insert(_Arg&&, const _NodeGenerator&, >> false_type)): Likewise. >> (_Hashtable(_InputIterator, _InputIterator, size_type, const >> _Hash&, const _Equal&, >> const allocator_type&, true_type)): Use this.insert range. >> (_Hashtable(_InputIterator, _InputIterator, size_type, const >> _Hash&, const _Equal&, >> const allocator_type&, false_type)): Use _M_insert. >> * testsuite/23_containers/unordered_map/cons/56112.cc: Check >> how many times conversion >> is done. >> (test02): New test case. >> * testsuite/23_containers/unordered_set/cons/56112.cc: New test. >> >> Tested under Linux x86_64. >> >> Ok to commit ? > No, sorry. > > The new test02 function in 23_containers/unordered_map/cons/56112.cc > doesn't compile with libc++ or MSVC either, are you sure that test is > valid? I don't think it is, because S2 is not convertible to > pair. None of the pair constructors are > viable, because the move constructor would require two user-defined > conversions (from S2 to pair and then from > pair to pair). A conversion > sequence cannot have more than one user-defined conversion using a > constructor or converion operator. So if your patch makes that > compile, it's a bug in the new code. I haven't analyzed that code to > see where the problem is, I'm just looking at the test results and the > changes in behaviour. > > The new 23_containers/unordered_set/cons/56112.cc test fails for GCC > 11 but passes for GCC 12, even without your patch. Is it actually > testing some other change, not this patch, and not the 2013 fix for PR > 56112? > > . Yes, I'm not surprised. This is due to this operator on _ValueTypeEnforcer:       constexpr __enable_if_t::value,                   const __mutable_value_t<_Value>&>       operator()(const __mutable_value_t<_Value>& __x) noexcept       { return __x; } I thought it was nice to allow move construction in this case. If the Standard forces a copy in this case I can just remove it. François