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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 8AB07389244B for ; Fri, 4 Jun 2021 20:44:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8AB07389244B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-351-lxTjCMibM8ua0L-J6haJuA-1; Fri, 04 Jun 2021 16:44:25 -0400 X-MC-Unique: lxTjCMibM8ua0L-J6haJuA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 07FBA1008545; Fri, 4 Jun 2021 20:44:24 +0000 (UTC) Received: from localhost (unknown [10.33.37.1]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F1EE5D9CC; Fri, 4 Jun 2021 20:44:23 +0000 (UTC) Date: Fri, 4 Jun 2021 21:44:22 +0100 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [committed] libstdc++: Fix value categories used by ranges access CPOs [PR 100824] Message-ID: References: MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="Rmy/s3mjTC6MmBNw" Content-Disposition: inline X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Fri, 04 Jun 2021 20:44:38 -0000 --Rmy/s3mjTC6MmBNw Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On 04/06/21 18:03 +0100, Jonathan Wakely wrote: >The implementation of P2091R0 was incomplete, so that some range access >CPOs used perfect forwarding where they should not. This fixes it by >consistently operating on lvalues. > >Some additional changes that are not necessary to fix the bug: > >Modify the __as_const helper to simplify its usage. Instead of deducing >the value category from its argument, and requiring callers to forward >the argument as the correct category, add a non-deduced template >parameter which is used for the value category and accept the argument >as an lvalue. This means callers say __as_const(t) instead of >__as_const(std::forward(t)). > >Always use an lvalue reference type as the template argument for the >_S_noexcept helpers, so that we only instantiate one specialization for >lvalues and rvalues of the same type. > >Move some helper concepts and functions from namespace std::__detail >to ranges::__cust_access, to be consistent with the ranges::begin CPO. >This ensures that the __adl_begin concept and the _Begin::operator() >function are in the same namespace, so unqualified lookup is consistent >and the poison pills for begin are visible to both. > >Simplified static assertions for arrays, because the expression a+0 is >already ill-formed for an array of incomplete type. > >Signed-off-by: Jonathan Wakely > >libstdc++-v3/ChangeLog: > > PR libstdc++/100824 > * include/bits/iterator_concepts.h (__detail::__decay_copy) > (__detail::__member_begin, __detail::__adl_begin): Move to > namespace ranges::__cust_access. > (__detail::__ranges_begin): Likewise, and rename to __begin. > Remove redundant static assertion. > * include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd): > Use lvalue in noexcept specifier. > (__as_const): Add non-deduced parameter for value category. > (_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of > __as_const. > (__member_size, __adl_size, __member_empty, __size0_empty): > (__eq_iter_empty, __adl_data): Use lvalue objects in > requirements. > (__sentinel_size): Likewise. Add check for conversion to > unsigned-like. > (__member_data): Allow non-lvalue types to satisfy the concept, > but use lvalue object in requirements. > (_Size, _SSize): Remove forwarding to always use an lvalue. > (_Data): Likewise. Add static assertion for arrays. > * testsuite/std/ranges/access/cdata.cc: Adjust expected > behaviour for rvalues. Add negative tests for ill-formed > expressions. > * testsuite/std/ranges/access/data.cc: Likewise. > * testsuite/std/ranges/access/empty.cc: Adjust expected > behaviour for rvalues. > * testsuite/std/ranges/access/size.cc: Likewise. An additional problem with ranges::data was pointed out in the PR, fixed with this patch. Tested powerpc64le-linux. Committed to trunk. This should also be backported to gcc-11 and gcc-10. --Rmy/s3mjTC6MmBNw Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 3e5f2425f80aedd00f28235022a2755eb46f310d Author: Jonathan Wakely Date: Fri Jun 4 20:25:39 2021 libstdc++: Fix helper concept for ranges::data [PR 100824] We need to decay the result of t.data() before checking if it's a pointer. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/100824 * include/bits/ranges_base.h (__member_data): Use __decay_copy. * testsuite/std/ranges/access/data.cc: Add testcase from PR. diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h index 17a421a4927..61d91eb8389 100644 --- a/libstdc++-v3/include/bits/ranges_base.h +++ b/libstdc++-v3/include/bits/ranges_base.h @@ -495,8 +495,10 @@ namespace ranges && is_object_v>; template - concept __member_data - = requires(_Tp& __t) { { __t.data() } -> __pointer_to_object; }; + concept __member_data = requires(_Tp& __t) + { + { __cust_access::__decay_copy(__t.data()) } -> __pointer_to_object; + }; template concept __begin_data = requires(_Tp& __t) diff --git a/libstdc++-v3/testsuite/std/ranges/access/data.cc b/libstdc++-v3/testsuite/std/ranges/access/data.cc index 237bbcc76c5..4f16f447f9f 100644 --- a/libstdc++-v3/testsuite/std/ranges/access/data.cc +++ b/libstdc++-v3/testsuite/std/ranges/access/data.cc @@ -92,8 +92,12 @@ test03() // ranges::data should treat the subexpression as an lvalue VERIFY( std::ranges::data(std::move(r)) == &R3::i ); VERIFY( std::ranges::data(std::move(c)) == &R3::l ); -} + // PR libstdc++/100824 comment 3 + // Check for member data() should use decay-copy + struct A { int*&& data(); }; + static_assert( has_data ); +} int main() --Rmy/s3mjTC6MmBNw--