c++implicit-conversionlvaluenoncopyable

Should Non-Copyable class have user conversion


My question is, should non-copyable objects have implicit/explicit user conversion? At least from my example below, conversions look very much like copies.

PS: I know that is recomended here to "Avoid implicit conversion operators" https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c164-avoid-implicit-conversion-operators

To explain my rationale suppose that I have a class that manages OS objects using RAII. Nothing fancy.

struct unique_handle
{
    unique_handle(HANDLE h) : handle(h) {}

    //NON COPYABLE
    unique_handle(const unique_handle&) = delete;
    unique_handle& operator=(const unique_handle&) = delete;

    //MOVEABLE
    unique_handle(unique_handle&& other) : handle(other.handle)
    {
        other.handle = INVALID_HANDLE_VALUE;
    }   
    unique_handle& operator=(unique_handle&& other)
    {
        if (this != &other) {
            handle = other.handle;
            other.handle = INVALID_HANDLE_VALUE;
        }
        return *this;
    }

    ~unique_handle()
    {
        if(handle != INVALID_HANDLE_VALUE)
            ::CloseHandle(handle);
    }
private:
    HANDLE handle;
}

Which is OK. It allows me to do something like:

namespace w32
{
    unique_handle CreateFile(...)       
    {
        HANDLE handle = ::CreateFileA(...);
        return { handle };
    }
}

The problem is that other OS functions will not accept my object. So I tried the, always dangerous, path of implicit/explicit user conversions.

struct unique_handle
{
...
    operator HANDLE() const noexcept { return handle; }
...
}

Which allows me to use my object normally.

...
auto dirHandle = w32::CreateFile(...); // my function returning my object
::ReadDirectoryChangesW(dirHandle, // my object on a native OS call
        ...);
...

Which is wonderful! But the problem now is that I allowed the possibility of leaking a closed handle. Suppose that I directly assign my object, never lvalue-ing it, to a HANDLE object, leaking not only the unmanaged HANDLE but even leaking a closed HANDLE. For example:

Problem 1
...
HANDLE h = w32::CreateFile(...);
...

I understand that the following happened on problem 1:

1 - I got the HANDLE from the OS and passed it to my object to managed it;
2 - I returned the HANDLE using the implicit user conversion operator;
3 - the compiler called my object destructor that closed the handle;
4 - a closed HANDLE has leaked in a non-obvious manner. Possible runtime error. Not even an error check is going to catch this because the HANDLE is not INVALID_HANDLE_VALUE.

Off course that I have also enabled this case. But for arguments sake, let us say that I accept this case.

Problem 2
HANDLE h1 = ...;
{
    auto h2 = w32::CreateFile(...); // my method and my object
    h1 = h2;                        // implicit cast to the OS HANDLE
}                                   // our favorite feature closing the HANDLE
::ReadDirectoryChangesW(h1, ...);
...

Because if, instead of opting for implicit/explicit user conversion, I opted for an unary operator, I could have avoided conversions from the temporary objects doing something like:

HANDLE operator !(unique_handle& v2)
{
    return v2.handle;
}

Which gives me a compilation error on lines such as:

...
HANDLE h = !w32::CreateFile(...);
...

Which is ideal, but as far I know we cannot do the same with conversions.

Other solutions that I imagine are functions such as:

struct unique_handle
{
    ...
    // just return - In my opinion also a copy, but what can we make?
    [[nodiscard]] HANDLE get() const noexcept { return handle; } 

    // Stop managing this HANDLE
    [[nodiscard]] HANDLE release() noexcept
    {
        auto h = handle;
        handle = INVALID_HANDLE_VALUE;
        return h;
    }
    ...
}

So going back to my question, is this scenario avoidable? Or Non-Copyable objects should never have user conversion? And to be honest, how are they different from a simple get() that returns some managed private object? Maybe a way to avoid user conversions from temporary objects? Can we force a object to be lvalue-ed before any other use, be it a method call or a conversion?


Solution

  • is this scenario avoidable?

    No. The library functions you're using do not, cannot and will not ever support your smart resource management. That's a shame, particularly as they do not provide any of their own, but it is the reality of the situation.

    Or Non-Copyable objects should never have user conversion? And to be honest, how are they different from a simple get() that returns some managed private object?

    .get() is a bigger warning flag to the reader that care must be taken regarding the lifetime of the resource being extracted in a "raw" fashion. Your implicit conversions are quicker and shorter to write, but do not signal this and are more likely to cause bugs.

    Maybe a way to avoid user conversions from temporary objects? Can we force a object to be lvalue-ed before any other use, be it a method call or a conversion?

    Yes, a member function can have ref-qualifiers. Although, as above, I don't think this really solves your overarching problem, which is that every use of these library functions will require careful observation by the reader, no matter what design you come up with.

    Look at std::unique_ptr. It has a get() function (to support third party functions that demand a T*) and no implicit conversions to anything (to prevent this from happening by accident).

    My general advice is to avoid implicit conversions where you can help it.