From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id E7D213857BA4; Tue, 7 Jun 2022 19:50:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E7D213857BA4 Received: by mail-wr1-x431.google.com with SMTP id p10so25544281wrg.12; Tue, 07 Jun 2022 12:50:56 -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:from:to:cc:references:in-reply-to :content-transfer-encoding; bh=0p5tyIQsstuH+THR5xT7nOgm5JSkSy3nEcZBwfRHKxE=; b=Q5z8NPWSQfSIpr2tfvCyXMXnmB74H3uMTbsMXYzgKc59L46mSnoBjOXb7TJe0BkvRb sywTbWlzlx6vlZUg5chtt8ebr2jqTazeVmvgZRX4b/AqMcYQcn8RHPySWFomdeijyxvP HpdgGxlPiRdDHEA/etzvIm56dfSbO4M4p9t5Z9pZQOkdeBm+Wda7Sgu6yfU3+Y4vBDtA fs48cVQjU6yWTvk1wjI/HOh/fQoNAtJw41LgCKuhzsXlt/AhPRX77YWFpqbt/5PUS4NR jNvm6NdD2voBOQKwsjlGyUQArBkQIN9mUgDKb9cytgR+auNOE194a1rrxRvsvG+1Xytl 5KUw== X-Gm-Message-State: AOAM5310zsgg2AtTLOCy9EIMWo7x3Os4sgHkUrcTZvTc6kCkyaV1EGGT RgUM9/RFNPYB8XGBDiJkwkI= X-Google-Smtp-Source: ABdhPJyX593wg/EyYfGzUf88bC80PNPaZZBqR04FWj/oLzC0UJ40YhsoiiYOHm+tuzmgr8AxROz+bg== X-Received: by 2002:a5d:52c7:0:b0:210:ac6:3956 with SMTP id r7-20020a5d52c7000000b002100ac63956mr29898611wrv.379.1654631455627; Tue, 07 Jun 2022 12:50:55 -0700 (PDT) Received: from ?IPV6:2a01:e0a:1dc:b1c0:f5f8:5ff4:b381:c45e? ([2a01:e0a:1dc:b1c0:f5f8:5ff4:b381:c45e]) by smtp.googlemail.com with ESMTPSA id y6-20020adfee06000000b0021004d7d75asm19316015wrn.84.2022.06.07.12.50.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Jun 2022 12:50:55 -0700 (PDT) Message-ID: Date: Tue, 7 Jun 2022 21:50:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH][_Hashtable] Fix insertion of range of type convertible to value_type PR 105714 Content-Language: fr From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <2fe3937d-1b9a-a547-bb41-225d3d5426a2@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_NUMSUBJECT, KAM_SHORT, 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, 07 Jun 2022 19:50:58 -0000 Hi Is this less ambitious version ok ? Thanks On 25/05/22 07:09, François Dumont wrote: > Here is the patch to fix just what is described in PR 105714. > >     libstdc++: [_Hashtable] Insert range of types convertible to > value_type PR 105714 > >     Fix insertion of range of types convertible to value_type. > >     libstdc++-v3/ChangeLog: > >             PR libstdc++/105714 >             * 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. >             * testsuite/23_containers/unordered_map/insert/105714.cc: > New test. >             * testsuite/23_containers/unordered_set/insert/105714.cc: > New test. > > Tested under Linux x64, ok to commit ? > > François > > On 24/05/22 12:31, Jonathan Wakely wrote: >> On Tue, 24 May 2022 at 11:22, Jonathan Wakely wrote: >>> On Tue, 24 May 2022 at 11: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. >>> I meant to include this link showing that libc++ and MSVC reject >>> test02() as well: >>> >>> https://godbolt.org/z/j7E9f6bd4 >> I've created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105717 for >> the insertion bug, rather than reopening PR 56112. >>