On Thu, Jun 24, 2021 at 6:28 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Wed, Jun 23, 2021 at 12:22 PM Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> Richard Biener <richard.guent...@gmail.com> writes:
> >> > On Wed, Jun 23, 2021 at 7:23 AM Trevor Saunders <tbsau...@tbsaunde.org> 
> >> > wrote:
> >> >>
> >> >> On Tue, Jun 22, 2021 at 02:01:24PM -0600, Martin Sebor wrote:
> >> >> > On 6/21/21 1:15 AM, Richard Biener wrote:
> >> > [...]
> >> >> > >
> >> >> > > But maybe I'm misunderstanding C++ too much :/
> >> >> > >
> >> >> > > Well, I guess b) from above means auto_vec<> passing to
> >> >> > > vec<> taking functions will need changes?
> >> >> >
> >> >> > Converting an auto_vec object to a vec slices off its data members.
> >> >> > The auto_vec<T, 0> specialization has no data members so that's not
> >> >> > a bug in and of itself, but auto_vec<T, N> does have data members
> >> >> > so that would be a bug.  The risk is not just passing it to
> >> >> > functions by value but also returning it.  That risk was made
> >> >> > worse by the addition of the move ctor.
> >> >>
> >> >> I would agree that the conversion from auto_vec<> to vec<> is
> >> >> questionable, and should get some work at some point, perhaps just
> >> >> passingauto_vec references is good enough, or perhaps there is value in
> >> >> some const_vec view to avoid having to rely on optimizations, I'm not
> >> >> sure without looking more at the usage.
> >> >
> >> > We do need to be able to provide APIs that work with both auto_vec<>
> >> > and vec<>, I agree that those currently taking a vec<> by value are
> >> > fragile (and we've had bugs there before), but I'm not ready to say
> >> > that changing them all to [const] vec<>& is OK.  The alternative
> >> > would be passing a const_vec<> by value, passing that along to
> >> > const vec<>& APIs should be valid then (I can see quite some API
> >> > boundary cleanups being necessary here ...).
> >>
> >> FWIW, as far as const_vec<> goes, we already have array_slice, which is
> >> mostly a version of std::span.  (The only real reason for not using
> >> std::span itself is that it requires a later version of C++.)
> >>
> >> Taking those as arguments has the advantage that we can pass normal
> >> arrays as well.
> >
> > It's not really a "const" thing it seems though it certainly would not 
> > expose
> > any API that would reallocate the vector (which is the problematic part
> > of passing vec<> by value, not necessarily changing elements in-place).
> >
> > Since array_slice doesn't inherit most of the vec<> API transforming an
> > API from vec<> to array_slice<> will be also quite some work.  But I
> > agree it might be useful for generic API stuff.
>
> Which API functions would be the most useful ones to have?  We could
> add them if necessary.

I think we'll see when introducing uses.  I guess that vec<> to const vec<>&
changes will be mostly fine but for vec<> to vec<>& I might prefer
vec<> to array_slice<> since that makes clear the caller won't re-allocate.
We'll see what those APIs would require then.

> There again, for things like searching and sorting, I think it would
> be better to use <algorithm> where possible.  It should usually be
> more efficient than the void* callback approach that the C code tended
> to use.

Not sure whether <algorithm> really is better, we've specifically replaced
libc qsort calls with our own sort to avoid all sorts of host isues and
the vec<>::bsearch is inline and thus the callbacks can be inlined.

Richard.

> Thanks,
> Richard

Reply via email to