This question is about how to implement assignment in modern C++ value types, with the goal of avoiding misleading code or code that is inconsistent with built-in behavior.
In C++, generated values (rvalue?) of built-in types are not assignable:
int generator_double() {
return 50;
}
...
generator_double() = 60; // error: expression is not assignable
This is probably a good thing, because it doesn't give the illusion that something can be done with the returned value.
However, naively designed user classes do not follow this rule.
struct UserClass {
int val_;
};
UserClass generator_user_class() {
return UserClass{99};
}
...
generator_user_class() = UserClass{66}; // compiles
This works and compiles, which can be misleading.
I guess this is allowed because operator= could have side effect that are not going to be discarded.
Or perhaps someone would like to pass this assigned value to a third function fun(generator_user_class1() = UserClass{66});, although for a pure value, this should be the same as fun(UserClass{66});.
So this inspires at least 3 or 4 different ways to implement assignments and incrementally improve on the possibly inconsistent initial version:
struct UserClass1 {
int val_;
UserClass1& operator=(UserClass1 const& other) & = default;
UserClass1& operator=(UserClass1 const& other) && = default;
};
UserClass1 generator_user_class1() { return UserClass1{99}; }
...
generator_user_class1() = UserClass1{66}; // compiles, can be misleading
struct UserClass2 {
int val_;
UserClass2& operator=(UserClass2 const& other) & = default;
UserClass2&& operator=(UserClass2 const& other) && = delete;
};
UserClass2 generator_user_class2() { return UserClass2{99}; }
...
generator_user_class2() = UserClass2{66};
struct UserClass3 {
int val_;
UserClass3& operator=(UserClass3 const& other) & = default;
[[nodiscard]] UserClass3&& operator=(UserClass3 const& other) && {
return std::move(operator=(other));
}
};
UserClass3 generator_user_class3() { return UserClass3{99}; }
...
generator_user_class3() = UserClass3{66}; // warns because of [[nodiscard]]
[[nodiscard]] and make all this work automatically, but it didn't help, it just forced me to implement operator= && to return by value.Here all the options in Godbolt to play. https://godbolt.org/z/h68hrxY19
So, What the correct modern way to implement assignment? Are there other idioms currently to handle this?
(For simplicity, the discussion can be restricted to types that are semantically values.)
EDIT 1:
I am told that option 2 is, in part, problematic because deleting some assignment operators is a bad idea and that this one is particular is deleted automatically because the line it has before. In this scenario, I am going to amend option 2.
2 bis.
struct UserClass2bis {
int val_;
UserClass2bis& operator=(UserClass2bis const& other) & = default;
// line removed: UserClass2bis&& operator=(UserClass2bis const& other) && = delete;
};
UserClass2bis generator_user_class2bis() { return UserClass2bis{99}; }
...
generator_user_class2bis() = UserClass2bis{66};
https://godbolt.org/z/9YbPb1rqo
EDIT 2
Both answers suggest ignoring this problem. One of the answers suggested that, if I do this at all, it would be preferable to do this:
const& to take care of all the non-& cases, && and const&&.struct UserClass2bisbis {
int val_;
UserClass2bisbis& operator=(UserClass2bisbis const& other) & = default;
UserClass2bisbis&& operator=(UserClass2bisbis const& other) const& = delete;
};
UserClass2bisbis generator_user_class2bisbis() { return UserClass2bisbis{99}; }
...
generator_user_class2bisbis() = UserClass2bisbis{66};
What the correct modern way to implement assignment?
Most modern guidelines agree that the correct way is to ignore the problem of assigning to an rvalue. Core Guidelines say:
C.20: If you can avoid defining default operations, do <...> Note This is known as “the rule of zero”.
Cppreference says:
Rule of zero
Classes that have custom destructors, copy/move constructors or copy/move assignment operators should deal exclusively with ownership (which follows from the Single Responsibility Principle). Other classes should not have custom destructors, copy/move constructors or copy/move assignment operators. [emphasis is mine]
But let's say you still want to catch "assignment to rvalue" bug. Let's consider your example, but start with std::vector<int> val_; member instead of int val;. I.e., start with:
struct UserClass {
std::vector<int> val_;
};
Then, your 1st way of fixing the problem loses move semantics:
struct UserClass2 {
std::vector<int> val_;
UserClass2& operator=(UserClass2 const& other) & = default;
UserClass2& operator=(UserClass2 const& other) && = delete;
};
The problem is that by defining copy-assignment operator, you suppressed move constructor and move assignment. You have to add them:
struct UserClass2 {
std::vector<int> val_;
UserClass2& operator=(UserClass2 const& other) & = default;
UserClass2& operator=(UserClass2&& other) & = default;
UserClass2(UserClass2&& other) = default;
};
Note that you do not need to delete any assignment operators: they are not generated because you have other assignment operators. I would probably follow "the rule of 5" and add 2 other operators even though they are not strictly needed:
struct UserClass2 {
std::vector<int> val_;
UserClass2& operator=(UserClass2 const& other) & = default;
UserClass2& operator=(UserClass2&& other) & = default;
UserClass2(UserClass2& other) = default;
UserClass2(UserClass2&& other) = default;
~UserClass2() = default;
};
Now, what about your UserClass3 example? You could add 2 more [[nodiscard]] operators for copy assignment and move assignment, but, in my opinion, adding so much code just to mark something as "no discard" is not worth it.