0
class Resource {
    Handle resource_handle;
public:
    friend void swap(Resource &a, Resource &b); // swap for the partial copy/swap idiom

    Resource(); // Default with uninitialized handle whose destruction is a noop
    Resource(std::string location); // Construction of resource (e.g. load something from disk)
    Resource(Resource &&other); // Move constructor to receive from returns of functions
    Resource &operator=(Resource other); // Sawp assignment to implement copy/swap idiom
    Resoruce(Resource &other) = delete; // You can not copy resources
    Resource &operator=(Resource &other) = delete; // You can not copy resources
};

A class managing a handle to a resource (file handles, gpu handles, mutexes) wants to prevent that the handle of the resoruce ever gets copied so the deconstruction of the wrapper class automatically frees the resource once and only once and nothing can access the handle anymore because the lifetime of the object has ended and (hopefully) no reference or pointer to the wrapper exists anymore.

The copy/swap and rule of 5(and a half) says that usually you want to define a copy constructor / assignment operator. Copying a resource handle is explicitly unwanted. Do I understand correctly that thus just deleting any other constructor / assignment operator solves this problem (and the compiler will shout at me if I ever assign something that is not converted to a rvalue (that therefore doesn't exist anymore after the assignment is done))

This is related to this question, as the resources I want to construct are actually only constructible after the containing data structure they are a member of is already constructed, making it necessary to move resources, but not copy them.

Parallel resource loading for OpenGL

salbeira
  • 2,375
  • 5
  • 26
  • 40
  • FYI: https://stackoverflow.com/questions/24342941/what-are-the-rules-for-automatic-generation-of-move-operations – engf-010 Apr 14 '21 at 15:42
  • 3
    If you **explicitly** declare them as `= delete` then they are available as an possible alternative, and if selected or ambiguous will cause a compilation error. However, if you allow the compiler to suppress them and never synthesize them, then they don't exist at all. Which is an important difference (sometimes The Right Thing, sometimes The Wrong Thing... depends on the need). – Eljay Apr 14 '21 at 15:44
  • Note - `Resource &operator=(Resource other); // Sawp assignment...` is going to swap into a temporary, probably not what you want to do. I would also use a `swap` member function instead to make the intention clear and delete the assignment operator. – Richard Critten Apr 14 '21 at 15:45
  • 1
    You want to use copy-and-swap with a noncopiable class? why? – Mooing Duck Apr 14 '21 at 15:53
  • Your approach is reasonable, except for operator=(Resource). You probably also want a move-assignment operator as well. (_Resource& operator=(Resource&& other)_) – Chris Uzdavinis Apr 14 '21 at 15:57
  • Copy-and-swap idiom became obsolete since C++11. Judging by the move constructor, you are using at least C++11. Furthermore, copy-and-swap is idiom for implementing a copy assignment operator. If you don't want your class to be copiable, then you shouldn't want to use copy-and-swap. – eerorika Apr 14 '21 at 16:17
  • @eerorika "*Copy-and-swap idiom became obsolete since C++11*" - not true. C&S is still quite alive, as not all types implement move semantics, only copy semantics. Especially when porting code from pre-C++11 compilers. – Remy Lebeau Apr 14 '21 at 17:00

1 Answers1

0

Deleting copy constructor and copy assignment operator for a resource handle class makes perfect sense and it would produce the desired result. (Note that copy constructor and copy assignment typically take a const reference argument. It doesn't matter here because the operator is deleted, but personally I always stick to a const reference unless there is a reason to do otherwise. I find that it makes the code easier to read.)

However the “swap assignment” is problematic.

First, it wouldn't work the way it's written in your example:

Resource &operator=(Resource other);  // bad

This functions takes an argument by value thus creating a copy. This wouldn't compile because copy constructor is deleted.

Second, even if something like this did work it would be misleading. People usually expect the right-hand side of an assignment to remain unchanged. I suggest to replace the “swap assignment” with a swap method:

void swap(Resource& other);

And then you can use this method to implement a non-member swap function, and it doesn't even need to be a friend function if the swap method is public.

Andrei Matveiakin
  • 1,558
  • 1
  • 13
  • 17
  • Wouldn't the copy still be possible if the copy was move constructed (aka ```Resource a = function(stuff);``` – salbeira Apr 14 '21 at 16:09
  • @salbeira `Resource a = function(stuff);` *may* invoke the move constructor from a temporary. But in modern C++ (ie C++17 onwards), it will actually elide the temporary altogether in most cases, allowing `function()` to construct `a` directly. – Remy Lebeau Apr 14 '21 at 17:04
  • "*This functions takes an argument by value thus creating a copy*" - not true. Since `Resource` has a move constructor, a caller can call the `operator=` with an rvalue as input, thus move-constructing `other` instead of copy-constructing it. In which case, it would be better to define `operator=` to take an rvalue reference as a parameter. But if `Resource` did support a copy constructor, then taking `other` by value is actually useful, allowing the caller to choose whether to copy-construct or move-construct `other` as needed, so you don't need separate `operator=` implementations. – Remy Lebeau Apr 14 '21 at 17:07
  • I guess the more relevant case is ```Resource a; a = function(stuff);``` because a resource wants to be a member of a containing class but needs to be constructed and initialized at a later date. e.g. what I want to be doing here: https://stackoverflow.com/questions/67096588/parallel-resource-loading-for-opengl – salbeira Apr 14 '21 at 17:57