From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id A9D173858C20 for ; Tue, 16 Aug 2022 07:59:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A9D173858C20 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 26B94113E; Tue, 16 Aug 2022 00:59:47 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.62]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B760D3F70D; Tue, 16 Aug 2022 00:59:45 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener , Martin Jambor , GCC Patches , richard.sandiford@arm.com Cc: Martin Jambor , GCC Patches Subject: Re: [09/23] Add a cut-down version of std::span (array_slice) References: Date: Tue, 16 Aug 2022 08:59:44 +0100 In-Reply-To: (Richard Biener's message of "Thu, 11 Aug 2022 08:58:24 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-51.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Aug 2022 07:59:48 -0000 Richard Biener writes: > On Wed, Aug 10, 2022 at 6:04 PM Martin Jambor wrote: >> >> Hello, >> >> I have one more question/comment about array_slice. Ever since I >> started to use it... >> >> On Fri, Nov 13 2020, Richard Sandiford via Gcc-patches wrote: >> > A later patch wants to be able to pass around subarray views of an >> > existing array. The standard class to do that is std::span, but it's >> > a C++20 thing. This patch just adds a cut-down version of it. >> > >> > The intention is just to provide what's currently needed. >> > >> > gcc/ >> > * vec.h (array_slice): New class. >> > --- >> > gcc/vec.h | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 120 insertions(+) >> > >> > diff --git a/gcc/vec.h b/gcc/vec.h >> > index f02beddc975..7768de9f518 100644 >> > --- a/gcc/vec.h >> > +++ b/gcc/vec.h >> > @@ -2128,6 +2128,126 @@ release_vec_vec (vec > &vec) >> > vec.release (); >> > } >> > >> > +// Provide a subset of the std::span functionality. (We can't use std::span >> > +// itself because it's a C++20 feature.) >> > +// >> > +// In addition, provide an invalid value that is distinct from all valid >> > +// sequences (including the empty sequence). This can be used to return >> > +// failure without having to use std::optional. >> > +// >> > +// There is no operator bool because it would be ambiguous whether it is >> > +// testing for a valid value or an empty sequence. >> > +template >> > +class array_slice >> > +{ >> > + template friend class array_slice; >> > + >> > +public: >> > + using value_type = T; >> > + using iterator = T *; >> > + using const_iterator = const T *; >> > + >> > + array_slice () : m_base (nullptr), m_size (0) {} >> > + >> > + template >> > + array_slice (array_slice other) >> > + : m_base (other.m_base), m_size (other.m_size) {} >> > + >> > + array_slice (iterator base, unsigned int size) >> > + : m_base (base), m_size (size) {} >> > + >> > + template >> > + array_slice (T (&array)[N]) : m_base (array), m_size (N) {} >> > + >> > + template >> > + array_slice (const vec &v) >> > + : m_base (v.address ()), m_size (v.length ()) {} >> > + >> > + iterator begin () { return m_base; } >> > + iterator end () { return m_base + m_size; } >> > + >> > + const_iterator begin () const { return m_base; } >> > + const_iterator end () const { return m_base + m_size; } >> > + >> > + value_type &front (); >> > + value_type &back (); >> > + value_type &operator[] (unsigned int i); >> > + >> > + const value_type &front () const; >> > + const value_type &back () const; >> > + const value_type &operator[] (unsigned int i) const; >> > + >> > + size_t size () const { return m_size; } >> >> ...this has been a constant source of compile errors, because vectors >> have length () and this is size (). >> >> I understand that the motivation was consistency with std::span, but do >> we really want to add another inconsistency with ourselves? >> >> Given that array_slice is not that much used yet, I believe we can still >> change to be consistent with vectors. I personally think we should but >> at the very least, if we keep it as it is, I'd like us to do so >> deliberately. > > We could alternatively add length in addition to size (and maybe size to > vec<> if std::vector has size but not length) with a comment deprecating > the "non-standard" variant? Yeah, I'd prefer to do the latter: add vec::size as a synonym of vec::length, and deprecate length. Doing anything else seems like it's going to increase the inconsistency rather than decrease it. E.g. we already have uses of (hopefully) uncontroversial standard containers like std::array (my fault). (FWIW, I keep tripping up in the opposite direction: expecting size to be available in vec, like for standard containers.) Thanks, Richard