[PATCH 5/6] make get_domminated_by_region return a auto_vec

[Richard Sandiford]

Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Wed, Jun 23, 2021 at 12:22 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener <richard.guenther@gmail.com> writes:
>> > On Wed, Jun 23, 2021 at 7:23 AM Trevor Saunders <tbsaunde@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.

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.

Thanks,
Richard