Is this swap implementation without destruction valid according to the standard?

Question

I propose this implementation of swap, if valid, is superior to the current implementation of std::swap:

#include <new>
#include <type_traits>

template<typename T>
auto swap(T &t1, T &t2) ->
    typename std::enable_if<
        std::is_nothrow_move_constructible<T>::value
    >::type
{
    alignas(T) char space[sizeof(T)];
    auto asT = new(space) T{std::move(t1)};
        // a bunch of chars are allowed to alias T
    new(&t1) T{std::move(t2)};
    new(&t2) T{std::move(*asT)};
}

The page for std::swap at cppreference, implies it uses move-assignment, because the noexcept specification depends on whether the move-assignment is no-throw. Furthermore, it has been asked here how is swap implemented and that is what I see in the implementations for libstdc++ and libc++

template<typename T>
void typicalImplementation(T &t1, T &t2)
    noexcept(
        std::is_nothrow_move_constructible<T>::value &&
        std::is_nothrow_move_assignable<T>::value
    )
{
    T tmp{std::move(t1)};
    t1 = std::move(t2);
        // this move-assignment may do work on t2 which is
        // unnecessary since t2 will be reused immediately
    t2 = std::move(tmp);
        // this move-assignment may do work on tmp which is
        // unnecessary since tmp will be immediately destroyed
    // implicitly tmp is destroyed
}

I dislike using move-assignment as in t1 = std::move(t2) because that implies executing the code to release the resources held in t1 if resources are held, even though it is known the resources in t1 where released already. I have a practical case in which releasing resources occur across a virtual method call, thus the compiler can't eliminate that unnecessary work because it can't know the virtual override code, whatever it is, won't do anything because there are no resources to release in t1.

If this is illegal in practice, could you please point out what it violates in the standard?

So far, I have seen in the answers and comments two objections that may make this illegal:

1. The ephemeral object created in tmp is not destructed, but there may be some assumption in user code that if a T is constructed it will be destructed
2. T may be a type with constants or references that can't be changed, the move assignment may be implemented swapping the resources without touching those constants or rebinding references.

Thus, it seems this construct is legal for any type except those that either hit case 1 or 2 above.

For illustration, I put a link to a compiler explorer page which shows all three implementations for the case of swapping vectors of ints, that is, the typical default implementation of std::swap, the specialized for vector, and the one I am proposing. You may see the proposed implementation performs less work than the typical, exactly the same as the specialized in the standard.

Only the user can decide to swap doing "all-move-construction" versus "one move construction, two move assignments", your answers inform users of then the "all-move-construction" is not valid.

After more side-band conversations with colleagues, what I am asking boils down to this works for types in which move can be seen as destructive, hence there is no need to balance the constructions with destructions.

Answer 1

It is illegal if T has ref or const members. Use std::launder or store the newed pointer (see [basic.life]p8)

auto asT = new(space) T{std::move(t1)};
// or use std::launder

But you also need to use std::launder for t1 and t2! And here you have a problem, because without std::launder, t1 and t2 refer to the old (already destructed) value and don't refer to the newly constructed object. Any access to t1 and t2 is then UB.

I dislike using move-assignment in swap because it implies the destruction of already-moved objects, which seems unnecessary work, hence this implementation.

Premature optimization? Now you need to call two destructors (t1 and t2)! Also, a destructor call is really not expensive.

Right now, as Nathan Oliver said, the destructors are not being called, (which is not UB), but you really shouldn't be doing that as the destructor might do important stuff.

Answer 2

Note that move constructors and assignment operators are required to leave their argument in a valid state. Typically, the implementation will default construct state then swap with the argument's state to steal its resources. Depending on the invariants the object wants to maintain, this may still leave the argument owning resources that it is relying on the destructor to reclaim. If destruction is elided, these will leak.

e.g. consider:

class X
{
public:
    X(): resource_( std::move( allocate_resource() ) )

    X( X&& other ): X()
    {
        std::swap( resource_, other.resource_ );
    }

private:
    std::shared_ptr<Y> resource_;
};

then,

X a;
X b;
swap( a, b );

Now, if swap is implemented as you propose, at the point where you do

new(&t2) T{std::move(*asT)};

we leak an instance of the resource, since the move constructor allocates one at *asT to replace the one it stole, and this is never destructed.

Another way of looking at this is that either the destruction does nothing, and so is cheap/free and so does not justify mystery-meat optimizations, or the destruction is expensive enough to care about, in which case it's doing something and so going behind the object's back to avoid doing that thing is morally wrong and will lead to badness down the line; fix the object implementation instead.