23

The question is about self-assignment. For example copying a vector into itself:

std::vector<std::string> vec(5, "hello");
vec = vec;

Should the code above perform 5 assignment operations of strings into themselves, or just do nothing? I mean whether the following checking is valid:

std::vector operator=(const std::vector &rhs)
{
    if (this == &rhs)
        { return *this; }
    ...
}

I'm working on my own implementation of std::variant class (just for fun) and interested if I should add a self-assignment check to the beginning of the assignment operator, or should I just copy the contained element into itself?

I understand that generally this doesn't matter. You should not make a class that utilizes the fact of copying into itself. But I'm interested if the standard says anything about this.

anton_rh
  • 8,226
  • 7
  • 45
  • 73
  • gcc keeps the code for assignment, but I can't tell if it has a self-assignment check first, while clang skips any work: https://godbolt.org/z/VdxN4D so I can't tell just from looking at the compiled code. Waiting for someone to go throught the standard and/or actual implementations. – bolov Aug 27 '19 at 10:31
  • 2
    This is also adviced in [C++ Core Guidelines](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c62-make-copy-assignment-safe-for-self-assignment). – ruohola Aug 27 '19 at 10:39
  • ruohola, the link you provided actually recommends not do a self-assignment check in a user class if all of its members are self-assign safe. See my [answer](https://stackoverflow.com/a/57676637/5447906) for details. – anton_rh Aug 27 '19 at 14:28
  • @bolov, both compilers skip self-assignments of the elements for `std::vector` (it's for self-assignment safety), but both perform self-assignment of the contained value for `std::variant` (I guess it's for efficiency): https://coliru.stacked-crooked.com/a/095d633dd908d53e – anton_rh Aug 27 '19 at 14:59
  • `if (this == &rhs) { return *this; }` is valid, but non-experts often use this to sidestep exception unsafe code. If removing this causes bugs, then the `operator=` was already buggy. It should _only_ be used as a performance optimization. – Mooing Duck Aug 27 '19 at 19:48

4 Answers4

18

The pre/post conditions of assignment of a container specified by the standard (quoting latest draft):

[tab:container.req]

r = a

Ensures: r == a.

This allows but does not mandate self assignment check.

Community
  • 1
  • 1
eerorika
  • 232,697
  • 12
  • 197
  • 326
  • 9
    To make the answer more complete: [utility.arg.requirements] (Table 29) specifies that the copy constructor does not change the argument, and the two values are equivalent. – L. F. Aug 27 '19 at 10:43
  • But what about a class that ensures that all its elements are unique? Should it be always not copy-assignable? – anton_rh Aug 27 '19 at 10:52
  • 1
    @anton_rh You cannot conditionally enable/disable copy assignment based on what variable you assign to what other variable (if the types are the same). There is no way to code "only enabled if `*this == other`" into the function signature. If your class does not support copy assignment in the general case, you cannot have a copy assignment operator. – Max Langhof Aug 27 '19 at 10:55
  • @L.F. I don't see directly why the copy constructor is relevant here? – Max Langhof Aug 27 '19 at 10:58
  • 1
    @MaxLanghof It is relevant because otherwise the element of a vector can be specially written so that copies compare unequal. – L. F. Aug 27 '19 at 10:59
  • @L.F. Oh, the copy constructor of the elements, not of the vector. I see, that makes sense. – Max Langhof Aug 27 '19 at 11:01
  • @L.F., does it mean that writing a class whose elements are unequal after assignment makes the program ill-formed or UB? Or I'm just not allowed to use it with STL container? – anton_rh Aug 27 '19 at 11:09
  • 5
    @anton_rh Not UB by itself, but indeed can't be used with most of the standard facilities. – L. F. Aug 27 '19 at 11:42
  • @L.F., in any case `std::vector` works [fine](https://coliru.stacked-crooked.com/a/d5ccfa80961e71a8) with a class that violates `r == a` post-condition. – anton_rh Aug 27 '19 at 15:17
  • 1
    @anton_rh It doesn’t really matter if it works “fine” or not, because the condition violator is the *user*, so in the perspective of the implementation, any result is *fine* - including producing the “intended” elements and spawning nasal demons. In this very case, I’d say any sane implementation would produce the result you observe, but the standard won’t agree :) – L. F. Aug 28 '19 at 04:13
4

interested if I should add a self-assignment check to the beginning of the assignment operator, or should I just copy the contained element into itself?

C++ Core Guidelines recommends not to do a self-assignment check in a user class if all of its members are self-assignment safe:

Enforcement (Simple) Assignment operators should not contain the pattern if (this == &a) return *this;???

It is for an efficiency reason. Self-assignments are unlikely to happen in practice. They are rare, and so it is better to avoid doing a self-assignment check in every operation. A self-assignment check probably makes the code faster in self-assignment case (very rare) and makes it slower in all other cases (more common).

Imagine you assign a million elements. In every assignment operation a self-assignment check is done. And it is most probably that it is done for nothing because none of the assignments is actually a self-assignment. And so we do a million useless checks.

If we skip doing a self-assignment check then nothing bad happens except that if self-assignment really happens then we do useless self-assignments of all members (that is sometimes slower than doing a single self-assignment check at the beginning of the assignment operator). But if your code does a million self-assignments it is a reason to reconsider your algorithm rather than to perform a self-assignment check in all of the operations.

However, self-assignment check still must be used for classes that are not self-assignment safe by default. The example is std::vector. The vector, that is being copied into, first has to delete the existing elements. But if the destination vector and source vector is the same object, then by deleting the elements in destination vector we also delete them in the source vector. And so it won't be possible to copy them after deletion. That is why libstdc++ does a self-assignment check for std::vector (though it is possible to implement std::vector without self-assignment check).

But it doesn't do it for std::variant for example. If you copy a variant into itself then the contained value will be copied into itself. See live example. Because copying it into itself is self-assignment safe (provided the contained value is self-assignment safe).

Thus, libstdc++ does a self-assignment check for std::vector (for providing self-assignment safety), and doesn't for std::variant (for efficiency).

anton_rh
  • 8,226
  • 7
  • 45
  • 73
  • 4
    Checking for self-assignment is not an efficiency concern. It protects the code from doing wrong things: say, a vector, when being copied into, first has to delete the existing elements, but if `this == &rhs`, it won't be able to copy from `rhs` after the elements are deleted. – lisyarus Aug 27 '19 at 14:46
  • @lisyarus, yes, it is called self-assignment safety. `std::vector` does this checking for the safety. But `std::variant` does not (I guess for efficiency reason): https://coliru.stacked-crooked.com/a/095d633dd908d53e – anton_rh Aug 27 '19 at 14:51
  • Right. I'd say what you've said in this answer applies perfectly to `std::variant`, though. – lisyarus Aug 27 '19 at 14:54
  • This answer doesn't seem to address the question. The question is about whether standard library containers do self-assignment checks but this answer makes recommendations for user classes. Also, answers to language-lawyer questions should be supported by references from the Standard. – M.M Aug 27 '19 at 21:56
1

[...] if I should add this checking to the beginning of the assignment operator [...] ?

You should, regardless whether std::vector, or other STL containers do that for you. Imagine a user that works with your library and does x = x, outside of STL containers scope.

Now to the STL container requirements - I believe the standard does not specify whether assignment is required to perform a check for being a self-assignment (went through the majority of Containers library section). This gives room for compiler optimisations and I believe that a decent compiler should perform such checks.

Fureeish
  • 12,533
  • 4
  • 32
  • 62
  • 6
    "compiler optimization" of self-assignment is irrelevant. Most cases of self-assignment happen when you have two references/pointers that you got from two different places that happen to be the same object. No compiler can detect that, since it's a runtime condition. – Nicol Bolas Aug 27 '19 at 14:43
0

Checking this == &rhs is actually a pretty well-known idiom, and helps to be sure that you don't break anything by guaranteeing that lhs and rhs are different objects. So, this is valid and actually encouraged.

I don't know whether STL containers are required to do the check, though.

lisyarus
  • 15,025
  • 3
  • 43
  • 68
  • @lisyarus: The question is tagged "language-lawyer" and specifically asks about the behavior of standard library containers, so implicit with that is the expectation that answers will specify what the C++ specification says on the subject. You said "*I don't know whether STL containers are required to do the check, though.*" That means you don't have the answer. Yes, it's a common idiom, but unless you can say that the standard library containers are required to use that idiom or not (or that the standard doesn't say), your answer isn't answering the question being asked. – Nicol Bolas Aug 27 '19 at 15:43
  • Anyway, it was helpful for me , because I didn't know such idiom. – anton_rh Aug 27 '19 at 15:55
  • @NicolBolas Thank you. I was specifically addressing the first part of the question, namely "whether the following checking is valid". – lisyarus Aug 27 '19 at 15:59
  • 2
    @NicolBolas Note that the language-lawyer tag was added by someone who was not the OP, and was added 10 minutes after lisyarus posted this answer. (Not to say that your comment wasn't good feedback, just that leaning heavily on the presence of a language-lawyer tag has issues in this case.) – R.M. Aug 27 '19 at 21:02