How to handle invalid state after move especially for objects with validating constructor?

user_9363996 0
Viewed 197

I made a class for a function's argument to delegate its validation and also for function overloading purposes.

Throwing from constructor guarantees that the object will either be constructed in a valid state or will not be constructed at all. Hence, there is no need to introduce any checking member functions like explicit operator bool() const.

// just for exposition
const auto &certString = open_cert();
add_certificate(cert_pem{certString.cbegin(), certString.cend()}); // this will either throw 
                                                                    // or add a valid certificate.
                                                                    // cert_pem is a temporary

However, there are issues which I don't see a appealing solution for: Argument-validation class might itself be made non-persistent - to be used only for validation as a temporary object. But what about classes that are allowed to be persistent? That is living after function invocation:

// just for exposition
const auto &certString = open_cert();

cert_pem cert{certString.cbegin(), certString.cend()}; // allowed to throw
cert_pem moved = std::move(cert); // cert invalidated
cert_pem cert_invalid = std::move(cert); // is not allowed to throw

add_certificate(cert_invalid); // we lost the whole purpoce

I can see several ways to treat this without introducing state-checking (thus declaring a class stateful) functions:

  1. Declare object "unusable" after move. - A really simple recipe for disaster
  2. Declare move constructor and assignment operator deleted. Allow only copy - Resources might be very expensive to copy. Or even not possible if using a PIMPL idiom.
  3. Use heap allocation when need an object to be persistent - this looks like most obvious. But has an unnecessary penalty on performance. Especially when some class has as members several such objects - there will be several memory allocations upon construction.

Here is a code example for 2):

/**
     * Class that contains PEM certificate byte array.
     * To be used as an argument. Ensures that input certificate is valid, otherwise throws on construction.
     */
    class cert_pem final
    {
    public:

        template <typename IterT>
        cert_pem(IterT begin, IterT end)
            : value_(begin, end)
        {
            validate(value_);
        }

        const std::vector<uint8_t>& Value() const noexcept(false)
        {
            return value_;
        }

        cert_pem (const cert_pem &) = default;
        cert_pem & operator=(const cert_pem &) = default;

        cert_pem (cert_pem &&) = delete;
        cert_pem & operator=(cert_pem &&) = delete;

    private:
        /**
         * \throws std::invalid_argument
         */
        static void Validate(const std::vector<uint8_t>& value) noexcept(false);
        static void ValidateNotEmpty(const std::vector<uint8_t>& value) noexcept(false);

    private:
        std::vector<uint8_t> value_;
    };

Is there another way to handle this problem without these shortcomings? Or will I have to choose one of the above?

I think that with argument-validating classes a good way would be to not allow it to be persistent - only temporary object is allowed. But I am not sure if it is possible in C++.

c++
2 Answers

You are trying to maintain two invariants at once, and their semantics are in conflict. The first invariant is the validity of the certificate. The second is for memory management.

For the first invariant, you decided that there can be no invalid constructed object, but for the second, you decided that the object can be either valid or unspecified†. This is only possible because the deallocation has a check somewhere.

There is no way around this: you either add a check for the first or you decouple the invariants. One way of decoupling them is to follow the design of std::lock_guard

cert c = open_cert(); // c is guaranteed to not have memory leaks and is movable
{
    cert_guard cg{c};  // cg is guaranteed to be valid, but cg is non-movable
}

But wait, you might ask, how do you transfer the validity to another cert_guard?

Well, you can't.

That is the semantics you chose for the first invariant: it is valid exactly during the lifetime of the object. That is the entire point.

† Unspecified and invalid as far as the certificate is concerned.

user_48324990

The question aims to design a type such that:

  1. an object of the type always satisfies a given invariant
  2. an object of the type is "usable" as a non-temporary

The question then makes a leap from (2) to ask that the type be movable. But it need not be: copy and move operations could be defined as deleted. The question fails to motivate why the move operations are necessary. If that is a need, it comes from an unstated requirement. A non-movable class can be emplaced in a map, returned from a function, and used in many other ways. It admittedly can be more painful to use, but it can be used.

So that's one option that's not listed: define copy and move operations as deleted.

Otherwise, let's assume we do want:

  1. an object of the type always satisfies a given invariant
  2. the type is movable

This is not in conflict. Every copyable class is movable, and copying is a valid strategy here. Remember that move operations allow a "potentially smarter" copy, by allowing the source to be mutated. There are still two C++ objects, and it is still a logical copy, but with an assumption that the source won't be needed anymore in its current state (so you can steal from it!). There is no difference in the C++ interface, only in the totally unchecked documented behavior of the type after a move operation.

Defining move operations as deleted gives you a copyable class. This is your second option listed. Assigning from an xvalue (cert_pem moved = std::move(cert)) will still compile, but will not invalidate the source. It will still be considered movable by the language. The trade-off is as you note, copies can be expensive. Note that PIMPL authors can give their types copy operations, that's a choice they make about what the interface of the type should be, and the idiom doesn't prevent it.

The third choice is a version of the second. By putting values behind a shared_ptr, one can make an expensive-to-copy type cheap to copy. But we still rely on copy as the strategy for move.

The first choice amounts to weakening the invariant in (1). A moved-from object satisfying a different set of invariants than a normal object is very typical in C++. It is annoying, but in many cases it is the best we can do. When only one object can exist satisfying the invariant (think: non-null unique_ptr) the moved-from object must violate it.

The accepted answer amounts to my first option combined with delayed construction: define copy and move operations as deleted. Creating the guard can throw if the object was moved-from. The guard is just the type maintaining the invariant, and it is non-movable. We can delay its construction because such types are difficult to manage. We do that by keeping an object that knows enough about how to construct it. This strategy exists in other forms (emplace functions and piecewise_construct constructors to construct objects in their eventual place, factory functions to construct the object at will, etc.).

However, the description in the accepted answer leaves a bit to be desired, in my opinion. The desire is to maintain the invariant while being movable (this is assumed). Being movable doesn't require that the moved-from object satisfy an invariant or be unspecified. That's a choice the author of the type makes, and what choices are available is exactly the question, by my reading of it. Although the example given only implicated memory, and the first answer mentioned memory, my reading of the question was more general: maintaining invariants in movable classes.

Knowing that all copyable classes are movable, that move is a "smart" copy, and that there are two objects in and after a move operation will help in understanding why there's such limited options here. One has to leave that source object in some state.

My advice is to embrace the radioactive moved-from object. That's the approach in the standard library, and defaulted move operations will obey that more often than not. For such types, there must be some "empty" state for moved-from objects, so all types are effectively optional and a default constructor can also be defined to get an object in that empty state.

user_32421460
Related