Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is the "used after it was moved [bugprone-use-after-move]" warning a real problem here?

I'm adapting the idea of the deffered ptr Herb Sutter talked about in cppcon 2016, to be able to manage external resources represented by an id in a safer way.

Therefore I created a non copy able and only movable class holding the id the resource should represent. Like a unique_ptr the id should become 0 if the object is moved to another object.

To my understanding you should still be allowed to use the object even after is was moved, if the called function does not have any preconditions, so to my understanding this should be valid:

int main() {

    resource src = make_resource(10);
    resource dst;
    std::cout << "src " << src.get() << std::endl;
    std::cout << "dst " << dst.get() << std::endl;

    dst = std::move(src);
    std::cout << "src " << src.get() << std::endl; // (*)
    std::cout << "dst " << dst.get() << std::endl;

    src = make_resource(40);
    std::cout << "src " << src.get() << std::endl;
    std::cout << "dst " << dst.get() << std::endl;

    return 0;
}

But clang-tidy give me this warning:

warning: 'src' used after it was moved [bugprone-use-after-move]

For the src.get() after the dst = std::move(src) (marked above).

So my questions are:

  1. Am I allowed to call src.get() after the std::move(src)
  2. May I make the assumption that src.get() returns 0 after the std::move.
  3. If 1. and 2. are valid, then is there a way to change the code so that clan-tidy knows that this is valid. And if not is there a way to change the code that it is valid?

Here is the implementation of the class:

struct resource {
    resource() = default;

    // no two objects are allowed to have the same id (prevent double free, only 0 is allowed multiple times as it represents nullptr)
    resource(const resource&) = delete;
    resource& operator=(const resource& other) = delete;

    // set the id of the object we move from back to 0 (prevent double free)
    resource(resource&& other) noexcept : id(std::exchange(other.id, 0)) {}
    resource& operator=(resource&& other) noexcept {
        id = std::exchange(other.id, 0);
        return *this;
    }

    // will free the external resource if id not 0
    ~resource() = default;

    // returns the id representing the external resource
    int get() const noexcept { return id; }

  protected:
    // only allow the make function to call the constructor with an id
    friend resource make_resource(int id);
    explicit resource(int id) : id(id) {}

  protected:
    int id = 0; // 0 = no resource referenced
};

// in the final version the id should be retrieved by from the external ip
resource make_resource(int id) { return std::move(resource(id)); }
like image 766
t.niese Avatar asked Dec 07 '22 13:12

t.niese


1 Answers

  1. Am I allowed to call src.get() after the std::move(src)

If we remain agnostic about the type of src, then we don't know. Potentially not. There are cases where calling a member function after move would be undefined. For example, calling the indirection operator of a smart pointer.

Given the definition of decltype(src) and its member functions that you've shown, we know: Yes, you're allowed to.

  1. May I make the assumption that src.get() returns 0 after the std::move.

If we remain agnostic about the type of src, we know nothing about what src.get() does. More specifically, we don't know its preconditions.

Given the definition of decltype(src) and its member functions that you've shown: Yes, we can make the assumption.

  1. If 1. and 2. are valid, then is there a way to change the code so that clan-tidy knows that this is valid. And if not is there a way to change the code that it is valid?

Clang-tidy assumes that "not being in a moved from state" is a precondition of all member functions (other than assignment), and under that assumption, is warning that such supposed precondition is being violated. As such, it is trying to enforce a convention to always assume such precodition even though you happen to know that it doesn't exist for your class.

You can remove the call to src.get() between the move, and the re-assignment of src. One operation on a moved from variable that clang-tidy doesn't complain about is re-assignment and after that assignment, the state of the object should (conventionally) be well defined and calling other member functions is assumed to be fine (of course, you can have other pre-conditions which must be met, but clang-tidy probably doesn't know of them). Although technically it would be possible to define a type for which even the assignment after move is not well defined, but such type would be quite unconventional and unsafe.


To conclude, you can call src.get() after move (even before reassignment) for this particular class, but then you won't be following the convention that clang-tidy is trying to enforce.

like image 105
eerorika Avatar answered May 16 '23 06:05

eerorika