From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 3E1083858C2D for ; Wed, 3 Aug 2022 15:13:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3E1083858C2D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 31F5E33E0F; Wed, 3 Aug 2022 15:13:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1659539609; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+JgJZPRunJlPM8ANVptxHMZbO740Eqg9trJojIkOg2w=; b=0wshQWVPV06+uen9f5/P4ZOeJc8j2wfexBM4Qs3NAtO6NckNJAtWln3ov9ecEaRabR5+3R NC5Fi/lMM24mZhZPPrtItw25uE4+/0soE6pk8txsyPp0WnOyldxFssRe4+92dbgt8iBPVW CL0r15nhkOe1XC0MQ63uLIcz69UvjWM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1659539609; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+JgJZPRunJlPM8ANVptxHMZbO740Eqg9trJojIkOg2w=; b=oenwxA0bXX1yj9btq9P8RKbeU6ynbbULA9ou+yRCN1bHcaTmceQ0qyYH8tfgnT7jXbUy8j +EZ2K0QzfGjEY/Dg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 25A1613A94; Wed, 3 Aug 2022 15:13:29 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id w1InCZmQ6mKsLwAAMHmgww (envelope-from ); Wed, 03 Aug 2022 15:13:29 +0000 From: Martin Jambor To: Richard Sandiford Cc: gcc-patches@gcc.gnu.org Subject: Re: [09/23] Add a cut-down version of std::span (array_slice) In-Reply-To: References: User-Agent: Notmuch/0.35 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Wed, 03 Aug 2022 17:13:28 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP 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: Wed, 03 Aug 2022 15:13:32 -0000 Hi Richard, 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. thanks a lot for introducing it. I hope to use it as a unified view into something which might be a GC vec or heap vec an an auto_vec. But I have one question: > > 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 ()) {} > + What is the reason for making the parameter const here? The problem is that if you do for example: auto_vec test_base; test_base.quick_grow_cleared (10); array_slice test(test_base); the constructor will get a const reference to test_base and so will invoke the const variant of v.address() which returns a const bool * which cannot be assigned into non-const qualified base. AFAICS, the constructor only works if the array_slice is array_slice. Is that intentional? I am not a C++ expert and can be easily overlooking something. I understand that users need to be careful not to cause reallocation of the underlying vector while the array_slice exists but the const qualifier does not achieve that. (A wild idea to be to add a array_slice ref-counter to auto_vec, which seems to be less space-efficiency-critical than other vecs, and assert on reallocation when it is not zero, hehe). Removing the const qualifier in the constructor parameter makes the error go away - as does adding another constructor without it, which might be the correct thing to do. On a related note, would the following constructor be a good addition to the class (I can make it const too)? template array_slice (vec *v) : m_base (v ? v->address () : nullptr), m_size (v ? v->length (): 0) {} Thanks, Martin > + 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; } > + size_t size_bytes () const { return m_size * sizeof (T); } > + bool empty () const { return m_size == 0; } > + > + // An invalid array_slice that represents a failed operation. This is > + // distinct from an empty slice, which is a valid result in some contexts. > + static array_slice invalid () { return { nullptr, ~0U }; } > + > + // True if the array is valid, false if it is an array like INVALID. > + bool is_valid () const { return m_base || m_size == 0; } > + > +private: > + iterator m_base; > + unsigned int m_size; > +}; > + > +template > +inline typename array_slice::value_type & > +array_slice::front () > +{ > + gcc_checking_assert (m_size); > + return m_base[0]; > +} > + > +template > +inline const typename array_slice::value_type & > +array_slice::front () const > +{ > + gcc_checking_assert (m_size); > + return m_base[0]; > +} > + > +template > +inline typename array_slice::value_type & > +array_slice::back () > +{ > + gcc_checking_assert (m_size); > + return m_base[m_size - 1]; > +} > + > +template > +inline const typename array_slice::value_type & > +array_slice::back () const > +{ > + gcc_checking_assert (m_size); > + return m_base[m_size - 1]; > +} > + > +template > +inline typename array_slice::value_type & > +array_slice::operator[] (unsigned int i) > +{ > + gcc_checking_assert (i < m_size); > + return m_base[i]; > +} > + > +template > +inline const typename array_slice::value_type & > +array_slice::operator[] (unsigned int i) const > +{ > + gcc_checking_assert (i < m_size); > + return m_base[i]; > +} > + > +template > +array_slice > +make_array_slice (T *base, unsigned int size) > +{ > + return array_slice (base, size); > +} > + > #if (GCC_VERSION >= 3000) > # pragma GCC poison m_vec m_vecpfx m_vecdata > #endif > -- > 2.17.1