Prevent std::move on object?

Question

I'm trying to create a not-null unique_ptr.

template <typename T>
class unique_ref {
public:
    template <class... Types>
    unique_ref(Types&&... Args) { mPtr = std::make_unique<T, Types...>(std::forward<Types>(Args)...); }
    T* release() && { return mPtr.release(); }
    T* release() & = delete;

private:
    std::unique_ptr<T> mPtr;
};

My goal is to allow release() only if the unique_ref is a temporary.

The problem is someone could use std::move() to "get around" this:

unique_ref<int> p;
int* p2 = std::move(p).release();

Is there a way to prevent it from being move'd?

I think you're worrying too much. If the caller has called `std::move`, you have to assume he knows what he's doing. — Richard Hodges, Apr 13 '19 at 01:03
@SidS above code does not invoke a move constructor at any point, so it would seem to me that that won't really achieve anything!? — Michael Kenzel, Apr 13 '19 at 01:03
@MichaelKenzel std::move will cast p to an r-value reference, which will cause the r-value overload of release to be selected. — Richard Hodges, Apr 13 '19 at 01:04
@MichaelKenzel, I was fooled by the title - he wants to prevent `move` of a pointer to object, not on the object. — Sid S, Apr 13 '19 at 01:05
@RichardHodges yes, but at no point will a move constructor be involved, so declaring a private move constructor, as suggested above, will not really do anything to prevent that!? — Michael Kenzel, Apr 13 '19 at 01:05
out of interest, if you don't want a user to call release(), why provide it? What's the real use case here? — Richard Hodges, Apr 13 '19 at 01:09
@RichardHodges it's to allow a function to return unique_ref without the caller needing to check for null. The released pointer could then be used to construct a shared_ptr or something else. — seattleite7, Apr 13 '19 at 01:16
The caller never _has_ to check for null. Just design your function to never return null and document accordingly. — Peter Ruderman, Apr 13 '19 at 01:19
It would be easier to not provide a `release()` method in the first place. Simply keep the `unique_ptr` alive for as long as the lifetime of the containing object. If they want to release, all they need to do is manage the lifetime of the object - i.e. normal practice, if they have any discipline, if they don't the only consequence is the pointee of the `unique_ptr` having a longer lifetime. — Peter, Apr 13 '19 at 02:20
See also https://stackoverflow.com/a/46303030/94687 , which shows how "a `value_ptr` can be written that is not nullable. (Note that there are extra costs at move.)" — imz -- Ivan Zakharyaschev, Feb 17 '20 at 06:15

score 2 · Answer 1 · answered Apr 13 '19 at 01:47

There is no way of distinguishing prvalues (temporaries) from xvalues (result of std::move) as far as overload resolution is concerned.

And there is no way of preventing std::move from converting an lvalue to an xvalue.

release is not an operation that can be supported by a non-null-guarantee "unique pointer". And neither is move construction / assignment. As far as I can tell, the only way to make the guarantee is to make the pointer non-movable, and make the copy operation allocate a deep copy.

score 2 · Answer 2 · answered Apr 13 '19 at 14:27

You're going to have to let the std::move case go. When a user invokes std::move, they are giving a strong signal that they know exactly what they are doing.

You can protect yourself though during debug time.

For example, I would consider starting the class definition a little like this:

#include <memory>
#include <cassert>

template <typename T>
class unique_ref {
public:
    // a number of problems here, but that is a discussuion for another day
    template <class... Types>
    unique_ref(Types&&... Args) 
    : mPtr(std::make_unique<T>(std::forward<Types>(Args)...))
    { }

    // unique_ref is implicitly move-only

    // see check below
    bool has_value() const {
        return bool(mPtr);
    }

    // here I am implicitly propagating the container's constness to the 
    // inner reference yielded. You may not want to do that.
    // note that all these accessors are marshalled through one static function
    // template. This gives me control of behaviour in exactly one place. 
    // (DRY principles)
    auto operator*() -> decltype(auto) {
        return *get_ptr(this);
    }

    auto operator*() const -> decltype(auto) {
        return *get_ptr(this);
    }

    auto operator->() -> decltype(auto) {
        return get_ptr(this);
    }

    auto operator->() const -> decltype(auto) {
        return get_ptr(this);
    }

private:
    using implementation_type = std::unique_ptr<T>;
    implementation_type release() { return std::move(mPtr); }

    // this function is deducing constness of the container and propagating it
    // that may not be what you want.
    template<class MaybeConst>
    static auto get_ptr(MaybeConst* self) -> decltype(auto)
    {
        auto ptr = self->mPtr.get();
        assert(ptr);
        using self_type = std::remove_pointer_t<decltype(self)>;
        if constexpr (std::is_const<self_type>())
            return static_cast<T const*>(ptr);
        else
            return ptr;
    }

private:
    implementation_type mPtr;
};

struct foo
{
};

auto generate()->unique_ref<foo> {
    return unique_ref<foo>();
}

void test()
{
    auto rfoo1 = generate();
    auto rfoo2 = generate();
//    auto rfoo3 = rfoo1; not copyable

    // we have to assume that a user knows what he's doing here
    auto rfoo3 = std::move(rfoo1);

    // but we can add a check
    assert(!rfoo1.has_value());

    auto& a = *rfoo3;
    static_assert(!std::is_const<std::remove_reference_t<decltype(a)>>());

    const auto rfoo4 = std::move(rfoo3);
    auto& b = *rfoo4;
    static_assert(std::is_const<std::remove_reference_t<decltype(b)>>());
}

Prevent std::move on object?

2 Answers2

Linked