From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AE5963858D28 for ; Tue, 14 Dec 2021 13:12:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AE5963858D28 Received: from mail-yb1-f198.google.com (mail-yb1-f198.google.com [209.85.219.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-578-qf-xymj_M-muFBPI9sS2XQ-1; Tue, 14 Dec 2021 08:12:41 -0500 X-MC-Unique: qf-xymj_M-muFBPI9sS2XQ-1 Received: by mail-yb1-f198.google.com with SMTP id y125-20020a25dc83000000b005c2326bf744so35950804ybe.21 for ; Tue, 14 Dec 2021 05:12:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kUoGmTGh9xbIalh4MU9A1PRC7NuE4u27rzQ3R7Sgkx8=; b=TxdNnROPcx4pduejm7CkqpGIKIik5xHuX9idL1NZg46/KFhMc2aWrC2Pnz8EujSEbY CgZAbXDs4nh6MvH/LgyzqvhmwwqJozn/IrRvZqIfK15lCrNyV+WqffQTy4Qz9tXjjWqe +Oi6tWww1wvpvR26xGV1GqrvVAAxyEKtip5LlpBks1WWFq7OKTUg94Kb288G59SGClys Y+jUqtHurNJQ+QhlzTNRSG8mPrUd8AgVMkPYh6Pq2F4yIHQXJSO0KpPJ30U4onmF1GiC 8WlWCD3s5Ioa/LRycTV+4lnMwz5O+PkWkIppSrQO720Y5whI+g2wev37cwB4YC2qfNgL cjsQ== X-Gm-Message-State: AOAM530kvR/RhccjfTSmx4lX4c9EDOkGOqptRf9O4weauaM0Ryb5eUj2 K1aTuEin60e/1THDrN0SLB8IDHtvf9x+KnMgRAbME9tDNUQ7xLb551krckmeUG6AYNTSkAHyt82 TCqEbjETZ12bpQbPxATd3yV1WRhGc04s= X-Received: by 2002:a25:f608:: with SMTP id t8mr5809208ybd.667.1639487561170; Tue, 14 Dec 2021 05:12:41 -0800 (PST) X-Google-Smtp-Source: ABdhPJzKbBSiefXi7+aqO/oda2ocg3XdEBl11troq9a+0X+UkZbMxS5VNkMULlZTbVTOfP3pkTKBLEvbPpgIrRHDFuQ= X-Received: by 2002:a25:f608:: with SMTP id t8mr5809181ybd.667.1639487560960; Tue, 14 Dec 2021 05:12:40 -0800 (PST) MIME-Version: 1.0 References: <57b54891-59ba-394d-3d53-218f5061b8ef@gmail.com> <9ea0770a-54fb-2c17-b527-2797b5fd3b50@gmail.com> <34b34b00-8c96-68c1-e436-ab0f64291c2f@gmail.com> <193032c8-8b9a-3e55-a244-075b0031d758@gmail.com> <5a951a74-5afd-a2da-02fd-9f9e7b33b448@gmail.com> In-Reply-To: <5a951a74-5afd-a2da-02fd-9f9e7b33b448@gmail.com> From: Jonathan Wakely Date: Tue, 14 Dec 2021 13:12:29 +0000 Message-ID: Subject: Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator> To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: Jonathan Wakely , "libstdc++" , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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, 14 Dec 2021 13:12:47 -0000 On Tue, 14 Dec 2021 at 06:53, Fran=C3=A7ois Dumont wrote: > Hi > > Any conclusion regarding this thread ? > > Fran=C3=A7ois > > > On 06/10/21 7:25 pm, Fran=C3=A7ois Dumont wrote: > > I forgot to ask if with this patch this overload: > > > > template > > constexpr auto > > __to_address(const _Ptr& __ptr, _None...) noexcept > > { > > if constexpr (is_base_of_v<__gnu_debug::_Safe_iterator_base, _Ptr= >) > > return std::__to_address(__ptr.base().operator->()); > > else > > return std::__to_address(__ptr.operator->()); > > } > > > > should be removed ? > > No, definitely not. That is the default overload for types that do not have a pointer_traits::to_address specialization. If you remove it, __to_address won't work for fancy pointers or any other pointer-like types. That would completely break it. The purpose of C++20's std::to_address is to get a real pointer from a pointer-like type. Using it with iterators is not the primary use case, but it needs to work with contiguous iterators because those are pointer-like. I made it work correctly with __normal_iterator because that was necessary to support the uses of std::__to_address in and , but I removed those uses in: https://gcc.gnu.org/g:247bac507e63b32d4dc23ef1c55f300aafea24c6 https://gcc.gnu.org/g:b83b810ac440f72e7551b6496539e60ac30c0d8a So now we don't really need the C++17 version of std::__to_address to work with __normal_iterator at all. I think it's OK to add the overload for __normal_iterator though, but only for C++11/14/17, because the default std::__to_address handles __normal_iterator correctly in C++20. > Or perhaps just the _Safe_iterator_base branch in it ? > Yes, you can just remove that branch, because your new overload handles it. > > > On 06/10/21 7:18 pm, Fran=C3=A7ois Dumont wrote: > >> Here is another proposal with the __to_address overload. > >> > >> I preferred to let it open to any kind of __normal_iterator > >> instantiation cause afaics std::vector supports fancy pointer types. > >> It is better if __to_address works fine also in this case, no ? > If we intend to support that, then we should verify it in the testsuite, using __gnu_test::CustomPointerAlloc. >> libstdc++: Overload std::__to_address for > >> __gnu_cxx::__normal_iterator. > >> > >> Prefer to overload __to_address to partially specialize > >> std::pointer_traits because > >> std::pointer_traits would be mostly useless. In the case of > >> __gnu_debug::_Safe_iterator > >> the to_pointer method is even impossible to implement correctly > >> because we are missing > >> the parent container to associate the iterator to. > To record additional rationale in the git history, please add that the partial specialization of pointer_traits<__normal_iterator> fails to rebind C, so you get incorrect types like __normal_iterator>. >> > >> libstdc++-v3/ChangeLog: > >> > >> * include/bits/stl_iterator.h > >> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove. > OK to remove this (it's broken anyway). >> (std::__to_address(const > >> __gnu_cxx::__normal_iterator<>&)): New. > Please make this only defined for C++11, 14 and 17. > >> * include/debug/safe_iterator.h > >> (std::__to_address(const > >> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, > >> _Sequence>&)): > >> New. > OK to add this (including for C++20), and remove the _Safe_iterator branch from the C++20 std::__to_address in . I think this new overload could return std::__to_address(__it.base().base()) though. That saves a function call, by going directly to the value stored in the __normal_iterator. > >> * testsuite/24_iterators/normal_iterator/to_address.cc: > >> Add check on std::vector::iterator > >> to validate both __gnu_cxx::__normal_iterator<> > >> __to_address overload in normal mode and the > Add similar checks for vector>. OK with those changes, thanks.