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 ESMTP id 6BF3439A1407 for ; Fri, 4 Jun 2021 20:46:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6BF3439A1407 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-98-IpIJmCd0OAOZWBMqgUcdWQ-1; Fri, 04 Jun 2021 16:46:10 -0400 X-MC-Unique: IpIJmCd0OAOZWBMqgUcdWQ-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 3AAA1100945E; Fri, 4 Jun 2021 20:46:09 +0000 (UTC) Received: from localhost (unknown [10.33.37.1]) by smtp.corp.redhat.com (Postfix) with ESMTP id D90CB5D9CC; Fri, 4 Jun 2021 20:46:08 +0000 (UTC) Date: Fri, 4 Jun 2021 21:46:08 +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="NZy/3fq/00AddNWd" 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:46:22 -0000 --NZy/3fq/00AddNWd Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On 04/06/21 21:44 +0100, Jonathan Wakely wrote: >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. And this implements the rest of LWG 3403. The change to the ranges::ssize constraints was already done by the first patch in this thread, this fixes the return type. Tested powerpc64le-linux. Committed to trunk. This should also be backported to gcc-11 and gcc-10. --NZy/3fq/00AddNWd Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 621ea10ca060ba19ec693aa73b5e29d553cca849 Author: Jonathan Wakely Date: Fri Jun 4 20:28:04 2021 libstdc++: Implement LWG 3403 for std::ranges::ssize I already changed the constraints for ranges::ssize to use ranges::size, this implements the rest of LWG 3403, so that the returned type is the signed type corresponding to the result of ranges::size. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: * include/bits/ranges_base.h (_SSize): Return the result of ranges::size converted to the wider of make-signed-like-t and ptrdiff_t, rather than the ranges different type. * testsuite/std/ranges/access/ssize.cc: Adjust expected result for an iota_view that uses an integer class type for its difference_type. diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h index 61d91eb8389..e3c3962bcd9 100644 --- a/libstdc++-v3/include/bits/ranges_base.h +++ b/libstdc++-v3/include/bits/ranges_base.h @@ -425,22 +425,32 @@ namespace ranges struct _SSize { + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 3403. Domain of ranges::ssize(E) doesn't match ranges::size(E) template requires requires (_Tp& __t) { _Size{}(__t); } constexpr auto operator()(_Tp&& __t) const noexcept(noexcept(_Size{}(__t))) { - using __iter_type = decltype(_Begin{}(__t)); - using __diff_type = iter_difference_t<__iter_type>; - using __gnu_cxx::__int_traits; auto __size = _Size{}(__t); - if constexpr (integral<__diff_type>) + using __size_type = decltype(__size); + // Return the wider of ptrdiff_t and make-signed-like-t<__size_type>. + if constexpr (integral<__size_type>) { - if constexpr (__int_traits<__diff_type>::__digits + using __gnu_cxx::__int_traits; + if constexpr (__int_traits<__size_type>::__digits < __int_traits::__digits) return static_cast(__size); + else + return static_cast>(__size); } - return static_cast<__diff_type>(__size); +#if defined __STRICT_ANSI__ && defined __SIZEOF_INT128__ + // For strict-ansi modes integral<__int128> is false + else if constexpr (__detail::__is_int128<__size_type>) + return static_cast(__size); +#endif + else // Must be one of __max_diff_type or __max_size_type. + return __detail::__max_diff_type(__size); } }; diff --git a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc index b00ec5db1f8..f53f462580c 100644 --- a/libstdc++-v3/testsuite/std/ranges/access/ssize.cc +++ b/libstdc++-v3/testsuite/std/ranges/access/ssize.cc @@ -76,9 +76,12 @@ void test06() { auto i = std::views::iota(1ull, 5u); - auto s = std::ranges::ssize(i); - using R = std::ranges::range_difference_t; - static_assert( std::same_as ); + auto s = std::ranges::size(i); + auto ss = std::ranges::ssize(i); + // std::ranges::range_difference_t is larger than long long, + // but LWG 3403 says ranges::ssize(i) returns the signed version of the + // type that ranges::size(i) returns, not the range's difference_type. + static_assert( std::same_as> ); VERIFY( s == 4 ); } --NZy/3fq/00AddNWd--