On Tue, Jun 1, 2021 at 9:56 PM Martin Sebor <mse...@gmail.com> wrote: > > On 5/27/21 2:53 PM, Jason Merrill wrote: > > On 4/27/21 11:52 AM, Martin Sebor via Gcc-patches wrote: > >> On 4/27/21 8:04 AM, Richard Biener wrote: > >>> On Tue, Apr 27, 2021 at 3:59 PM Martin Sebor <mse...@gmail.com> wrote: > >>>> > >>>> On 4/27/21 1:58 AM, Richard Biener wrote: > >>>>> On Tue, Apr 27, 2021 at 2:46 AM Martin Sebor via Gcc-patches > >>>>> <gcc-patches@gcc.gnu.org> wrote: > >>>>>> > >>>>>> PR 90904 notes that auto_vec is unsafe to copy and assign because > >>>>>> the class manages its own memory but doesn't define (or delete) > >>>>>> either special function. Since I first ran into the problem, > >>>>>> auto_vec has grown a move ctor and move assignment from > >>>>>> a dynamically-allocated vec but still no copy ctor or copy > >>>>>> assignment operator. > >>>>>> > >>>>>> The attached patch adds the two special functions to auto_vec along > >>>>>> with a few simple tests. It makes auto_vec safe to use in containers > >>>>>> that expect copyable and assignable element types and passes > >>>>>> bootstrap > >>>>>> and regression testing on x86_64-linux. > >>>>> > >>>>> The question is whether we want such uses to appear since those > >>>>> can be quite inefficient? Thus the option is to delete those > >>>>> operators? > >>>> > >>>> I would strongly prefer the generic vector class to have the properties > >>>> expected of any other generic container: copyable and assignable. If > >>>> we also want another vector type with this restriction I suggest to add > >>>> another "noncopyable" type and make that property explicit in its name. > >>>> I can submit one in a followup patch if you think we need one. > >>> > >>> I'm not sure (and not strictly against the copy and assign). Looking > >>> around > >>> I see that vec<> does not do deep copying. Making auto_vec<> do it > >>> might be surprising (I added the move capability to match how vec<> > >>> is used - as "reference" to a vector) > >> > >> The vec base classes are special: they have no ctors at all (because > >> of their use in unions). That's something we might have to live with > >> but it's not a model to follow in ordinary containers. > > > > I don't think we have to live with it anymore, now that we're writing > > C++11. > > > >> The auto_vec class was introduced to fill the need for a conventional > >> sequence container with a ctor and dtor. The missing copy ctor and > >> assignment operators were an oversight, not a deliberate feature. > >> This change fixes that oversight. > >> > >> The revised patch also adds a copy ctor/assignment to the auto_vec > >> primary template (that's also missing it). In addition, it adds > >> a new class called auto_vec_ncopy that disables copying and > >> assignment as you prefer. > > > > Hmm, adding another class doesn't really help with the confusion richi > > mentions. And many uses of auto_vec will pass them as vec, which will > > still do a shallow copy. I think it's probably better to disable the > > copy special members for auto_vec until we fix vec<>. > > There are at least a couple of problems that get in the way of fixing > all of vec to act like a well-behaved C++ container: > > 1) The embedded vec has a trailing "flexible" array member with its > instances having different size. They're initialized by memset and > copied by memcpy. The class can't have copy ctors or assignments > but it should disable/delete them instead. > > 2) The heap-based vec is used throughout GCC with the assumption of > shallow copy semantics (not just as function arguments but also as > members of other such POD classes). This can be changed by providing > copy and move ctors and assignment operators for it, and also for > some of the classes in which it's a member and that are used with > the same assumption. > > 3) The heap-based vec::block_remove() assumes its elements are PODs. > That breaks in VEC_ORDERED_REMOVE_IF (used in gcc/dwarf2cfi.c:2862 > and tree-vect-patterns.c). > > I took a stab at both and while (1) is easy, (2) is shaping up to > be a big and tricky project. Tricky because it involves using > std::move in places where what's moved is subsequently still used. > I can keep plugging away at it but it won't change the fact that > the embedded and heap-based vecs have different requirements.
So you figured that neither vec<> nor auto_vec<> are a container like std::vector. I'm not sure it makes sense to try to make it so since obviously vec<> was designed to match the actual needs of GCC. auto_vec<> was added to make a RAII (like auto_bitmap, etc.) wrapper, plus it got the ability to provide initial stack storage. > It doesn't seem to me that having a safely copyable auto_vec needs > to be put on hold until the rats nest above is untangled. It won't > make anything worse than it is. (I have a project that depends on > a sane auto_vec working). So how does your usage look like? I can't really figure who'd need deep copying of a container - note there's vec<>::copy at your discretion. > A couple of alternatives to solving this are to use std::vector or > write an equivalent vector class just for GCC. As said, can you show the usage that's impossible to do with the current vec<>/auto_vec<>? Richard. > Martin > > > > > Jason > > >
