On Thu, Jun 24, 2021 at 12:56 AM Martin Sebor <mse...@gmail.com> wrote: > > On 6/23/21 1:43 AM, Richard Biener wrote: > > 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 ...). > > > > But with all this I don't know how to adjust auto_vec<> to no > > longer "decay" to vec<> but still being able to pass it to vec<>& > > and being able to call vec<> member functions w/o jumping through > > hoops. Any hints on that? private inheritance achieves the first > > but also hides all the API ... > > The simplest way to do that is by preventing the implicit conversion > between the two types and adding a to_vec() member function to > auto_vec as Jason suggested. This exposes the implicit conversions > to the base vec, forcing us to review each and "fix" it either by > changing the argument to either vec& or const vec&, or less often > to avoid the indirection if necessary, by converting the auto_vec > to vec explicitly by calling to_vec(). The attached diff shows > a very rough POC of what that might look like. A side benefit > is that it improves const-safety and makes the effects of functions > on their callers easier to understand. > > But that's orthogonal to making auto_vec copy constructible and copy > assignable. That change can be made independently and with much less > effort and no risk.
There's a bunch of stuff I can't review because of lack of C++ knowledge (the vNULL changes). I suppose that + /* Prevent implicit conversion from auto_vec. Use auto_vec::to_vec() + instead. */ + template <size_t N> + vec (auto_vec<T, N> &) = delete; + + template <size_t N> + void operator= (auto_vec<T, N> &) = delete; still allows passing auto_vec<> to [const] vec<> & without the .to_vec so it's just the by-value passing that becomes explicit? If so then I agree this is an improvement. The patch is likely incomplete though or is it really only so few signatures that need changing? Thanks, Richard. > Martin
