[cppcoreguidelines-special-member-functions] Spiritually false positive for `operator=(T)`

[Eisenwave]

Consider the example (https://godbolt.org/z/fPP6cnbYv):

struct MyClass {
    MyClass(const MyClass&);
    MyClass(MyClass&&);
    ~MyClass();
    MyClass& operator=(MyClass other) noexcept;
};

<source>:1:8: warning: class 'MyClass' defines a destructor, a copy constructor, a copy assignment operator and a move constructor but does not define a move assignment operator [cppcoreguidelines-special-member-functions]
    1 | struct MyClass {
      |        ^

Technically, clang-tidy is right, and MyClass does not have a move assignment operator.

However, this is spiritually a false positive because operator=(MyClass) accepts both rvalues and lvalues, and is acting as a "unified" assignment operator. clang-tidy should not emit a diagnostic in the event that the copy assignment operator has a by-value parameter, or this behavior should be configurable somehow.

See Stack Overflow/clang-tidy does not recognize unifying assignment operator as move assigment for more discussion.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Jan Schultke (Eisenwave)

Consider the example (https://godbolt.org/z/fPP6cnbYv): ```cpp struct MyClass { MyClass(const MyClass&); MyClass(MyClass&&); ~MyClass(); MyClass& operator=(MyClass other) noexcept; }; ``` ```none <source>:1:8: warning: class 'MyClass' defines a destructor, a copy constructor, a copy assignment operator and a move constructor but does not define a move assignment operator [cppcoreguidelines-special-member-functions] 1 | struct MyClass { | ^ ``` Technically, clang-tidy is right, and `MyClass` does not have a move assignment operator. However, this is spiritually a false positive because `operator=(MyClass)` accepts both rvaues and lvalues and is acting as a "unified" assignment operator. clang-tidy should not emit a diagnostic in the event that the copy assignment operator has a by-value parameter, or this behavior should be configurable somehow. See [Stack Overflow/clang-tidy does not recognize unifying assignment operator as move assigment](https://stackoverflow.com/q/77692186/5740428) for more discussion.

[overmighty]

Rule C.21 of the C++ Core Guidelines is about defining "either all or none of the copy/move/destructor functions" specifically, not just functions that can be called with both rvalues and lvalues.

[Eisenwave]

@overmighty if you use the copy-and-swap idiom, it's not actually possible to follow this guideline. You cannot define all, like the guideline suggests.

• You cannot declare operator=(T&&) = default since you're doing manual resource management, and this would be an ambiguous overload even if it did work.
• You cannot declare operator=(T&&) = delete since once again, you get ambiguous overloads.
• You cannot declare operator=(T&&) { ... } since that would defeat the whole point of having a by-value copy assignment operator. Also, ambiguous overload.

Since it's impossible to follow this rule in that specific case, there needs to be some extra lenience in clang-tidy.

[overmighty]

@Eisenwave Since this Clang-Tidy check was made to enforce C.21, I believe that it shouldn't make any exceptions, as C.21 doesn't state any. If there are only rare cases where you do not want this diagnostic to be emitted, then you can simply suppress it for each case like this:

struct MyClass { // NOLINT(cppcoreguidelines-special-member-functions)
    MyClass(const MyClass&);
    MyClass(MyClass&&);
    ~MyClass();
    MyClass& operator=(MyClass other) noexcept;
};

or:

// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions)
struct MyClass {
    MyClass(const MyClass&);
    MyClass(MyClass&&);
    ~MyClass();
    MyClass& operator=(MyClass other) noexcept;
};

[Eisenwave]

@overmighty I think C.21 simply doesn't have an exception because no one thought of this case, or no one bothered to state it explicitly. I've opened an issue in CppCoreGuidelines; perhaps the exception will added and the decision on this issue will be obvious.

Thanks for the suggested workaround; it's the best I've seen so far.