[Bug libstdc++/98473] New: std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[Bug libstdc++/98473] New: std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[b.stanimirov at abv dot bg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98473

            Bug ID: 98473
           Summary: std::vector<T>::insert(pos, first, last) doesn't
                    compile for T which has a deleted assignment operator
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: b.stanimirov at abv dot bg
  Target Milestone: ---

Create a class `X` which is copy-constructible but not copy-assignable

```
struct X {
    X();
    X(const X&);
    X& operator=(const X&) = delete; // !!
    X(X&&) noexcept;
    X& operator=(X&&) noexcept;
    int data = 54;
};
```

Have vectors of X `a` and `b`. Try to add all elements of b at the front of a:

```
void add_to_front(std::vector<X>& a, const std::vector<X>& b) {
    a.insert(a.begin(), b.begin(), b.end());
}
```

Live demo: https://godbolt.org/z/K1WT8n

It doesn't compile. My guess is that code which will never get invoked, still
needs to compile (or, worse yet, copy assignment does get invoked?!)

The only way to achieve the desired behavior efficiently is to use a custom
vector implementation. There is a stackoverflow question which has some
workarounds *with worse performance*:
https://stackoverflow.com/questions/65489039/how-to-efficiently-insert-multiple-copy-constructible-but-not-copy-assignable-el

This does compile and work on msvc, so a compile-time check for the
copy-assignment code is possible.

As pointed out in the stackoverflow thread, copy-assignability is not listed
here: https://en.cppreference.com/w/cpp/container/vector/insert#Parameters

This looks like a bug in libstdc++ (as opposed to a omission by the standard)

[Bug libstdc++/98473] std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98473

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Complete example:

#include <vector>

struct X {
    X();
    X(const X&);
    X& operator=(const X&) = delete; // !!
    X(X&&) noexcept;
    X& operator=(X&&) noexcept;
    int data = 54;
};

void add_to_front(std::vector<X>& a, const std::vector<X>& b) {
    a.insert(a.begin(), b.begin(), b.end());
}

(In reply to Borislav Stanimirov from comment #0)
> It doesn't compile. My guess is that code which will never get invoked,
> still needs to compile (or, worse yet, copy assignment does get invoked?!)

When a.capacity() > a.b.size() the elements from b are copied over existing
elements of a using std::copy, which uses copy assignment.

To meet the requirements of the standard we would need to insert them at the
end and then use std::rotate to reposition them.

I see that libc++ has the same behaviour as libstdc++, which does make me think
the standard is wrong to require this (since it doesn't reflect reality). I'll
have to check the history of the standard.

> This does compile and work on msvc, so a compile-time check for the
> copy-assignment code is possible.

No, you can't check that at compile time. It's a dynamic condition. Presumably
MSVC uses std::rotate instead so no check is needed.

[Bug libstdc++/98473] std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[b.stanimirov at abv dot bg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98473

--- Comment #2 from Borislav Stanimirov <b.stanimirov at abv dot bg> ---
(In reply to Jonathan Wakely from comment #1)

> To meet the requirements of the standard we would need to insert them at the
> end and then use std::rotate to reposition them.

Or, to save move assignments, first "make a hole" and then copy-construct b's
in place (some destructors of moved-out objects) will have to be called. 

> 
> I see that libc++ has the same behaviour as libstdc++, which does make me
> think the standard is wrong to require this (since it doesn't reflect
> reality). I'll have to check the history of the standard.
> 

I reported the same issue there too:
https://bugs.llvm.org/show_bug.cgi?id=48619

> > This does compile and work on msvc, so a compile-time check for the
> > copy-assignment code is possible.
> 
> No, you can't check that at compile time. It's a dynamic condition.
> Presumably MSVC uses std::rotate instead so no check is needed.

You definitely can check whether the class has a copy assignment operator and
then, based on that, choose either the current implementation or one which
doesn't invoke it (rotate or other)

[Bug libstdc++/98473] std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[b.stanimirov at abv dot bg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98473

--- Comment #3 from Borislav Stanimirov <b.stanimirov at abv dot bg> ---
By the way, this is not just some esoteric synthetic example. A type which is
copy-constructible but not copy-assignable is very useful to model immutable
objects.

[Bug libstdc++/98473] std::vector<T>::insert(pos, first, last) doesn't compile for T which has a deleted assignment operator

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98473

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Borislav Stanimirov from comment #2)
> (In reply to Jonathan Wakely from comment #1)
> 
> > To meet the requirements of the standard we would need to insert them at the
> > end and then use std::rotate to reposition them.
> 
> Or, to save move assignments, first "make a hole" and then copy-construct
> b's in place (some destructors of moved-out objects) will have to be called.

If any of those copy constructions throws then you have a vector with holes in
it, breaking the invariant. Making that exception safe makes the code more
complicated (and slower) which is why inserting them at the end and rotating is
a better choice.