How risky is it to destroy_at/construct non-copyable vector elements?

user_4987285 0
Viewed 157

I've searched through SO and have a question about whether the following is safe or not.

Consider this vector:

std::vector<std::pair<const key, value>> vec;

Imagine that I want to swap-and-pop an element at position i with the last one. Ignore for a moment the fact that if something goes wrong the vector may be in an unsafe state. I perfectly know that but the question is more about the other operation around. Since I can't swap the two elements, I'd be tempted to do this:

auto *elem = std::addressof(vec[i]);
std::destroy_at(elem);
std::construct(alloc, elem, std::move(vec.back());
vec.pop_back();

Again, we are in an unsafe state between line 2 and 3 but let's ignore it. I'm curious to know if destroying and reconstructing the element at position i is safe for all i instead.

From my understanding, I've a couple of doubts about it:

  • When i is 0, we're somewhat playing with the same initial pointer as stored by the vector itself. Therefore, I wonder if in this case we should force the vector to std::launder it (yeah, it's not possible, I'm just enetering the in theory field here).

  • Since the pair has a const key, I guess destroying and recostructing it can lead to UB. Though, I'm not that sure about this, just my gut feeling.

c++17 undefined-behavior language-lawyer c++
2 Answers

At first, I'm only referring to this question of you with a look onto your const-members concerns and onto the issue without the context of std::vector and std::construct_at (std::construct requires the allocator further on...):

Since the pair has a const key, I guess destroying and recostructing it can lead to UB. Though, I'm not that sure about this, just my gut feeling.

Before C++20, you're right, this would be UB if old references and pointers are still on use. But as of C++20, there's a relevant change here in basic.life#8.3 to n4861/basic.life#8.3

If, after the lifetime of an object has ended and before the storage which the object occupied is reused or released, a new object is created at the storage location which the original object occupied, a pointer that pointed to the original object, a reference that referred to the original object, or the name of the original object will automatically refer to the new object and, once the lifetime of the new object has started, can be used to manipulate the new object, if the original object is transparently replaceable (see below) by the new object. An object o1 is transparently replaceable by an object o2 if

  1. the storage that o2 occupies exactly overlays the storage that o1 occupied, and
  2. o1 and o2 are of the same type (ignoring the top-level cv-qualifiers), and
  3. o1 is not a complete const object, and
  4. neither o1 nor o2 is a potentially-overlapping subobject, and
  5. either o1 and o2 are both complete objects, or o1 and o2 are direct subobjects of objects p1 and p2, respectively, and p1 is transparently replaceable by p2.

So as of these changes, you should actually be fine in general for your particular(!) example since vector elements have to be non-const, i.e. the referred objects cannot be const complete ones (subpoint 3). Keep in mind that these phrases are relevant for the case of further using old references and pointers to this reused location! A simple placement new after an explicit destruction (not de-allocation) is fine a priori, it's the further context, that's relevant for the question about UB.

Also see the discussion from

Is it possible now with the current C++ standard draft version to define a copy assignment operator for classes with const fields without UB

Taking the std::vector-context into account with std::construct_at except the zero position case:

Your concerns about constant members are actually not longer relevant here since we have an in-between layer: The allocator (of std::vector) and the promise of simple contiguous storage and the fact, that there are no "pending" references to the old memory (as long as you do not refer to your old elem pointer further on...). Otherwise, a lot of highly efficient operations on/inside the standard vector container would not be possible (emplace data into pre-allocated ranges). The standard library itself uses similar schemes for moving and insertion for instance for many containers types. It's still very ugly to circumvent the inner memory "assumptions" of an external container this way but it should be ok for all common actual library implementations of an std::vector (but surely not for containers with complex inner logic like a map!).

user_145691060

This is not UB but it opens pitfalls. Before C++20 all pointers and references to vec[i] are gone at destroy time so using them after the operation would be UB.

And even in C++20 pointers or references to vec[i].first still go away, because vec[i].first is a complete const object, so it is not transparently replaceable.

It is not worse than using pop_back and then push_back or emplace_back to replace the last element of a vector. But it has the exact same caveats: you have a different object at the place where a former object existed. So using a reference or pointer to anything up to C++17, or to anything that is declared as const since C++20, is explicitely UB.

The underlying reason is that optimizing compilers are fond of caching anything that is declared as const. So any trick ending in replacing a const object is only safe if you are sure to never use a direct pointer, reference or variable that refered to that damned const thing.

It is rather evident when you play with the tail of a container, because future users of that code are aware that something has disappeared. I believe that doing that in the middle of a vector is more dangerous, because it could easily mislead future maintainers or users. At least it requires a warning comment in red flashing font.

user_35452730
Related