Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Warning: defaulted move assignment operator of X will move assign virtual base class Y multiple times

I'm catching a warning under Clang when testing a library under C++11. I've never come across the warning before and searching is not providing too much in the way of reading and research.

The warning is shown below, and it appears to be related to multiple inheritance and a common base class. But I'm not clear on the details triggering the warning or what I should do to address it.

My first question is, Is this a problem that needs to be addressed? Or is it a matter of efficiency alone?

My second question is (if needed), How do I address the warning? Or what are the options available to remediate it?


Here is some additional information:

  • Compiler: Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)
  • g++ -DDEBUG -g2 -O2 -std=c++11 -fPIC -march=native -pipe -c test.cpp

Is also reviewed the following on Stack Overflow, but its not clear to me where they intersect:

  • Rule-of-Three becomes Rule-of-Five with C++11?

The library, Crypto++, also make heavy use of Curiously Recurring Template Pattern for compile time polymorphism.


The the header file is available online, and here is the actual warning:

g++ -DDEBUG -g2 -O2 -std=c++11  -Wno-deprecated-declarations -fPIC -march=native -pipe -c rsa.cpp
In file included from rsa.cpp:4:
In file included from ./rsa.h:12:
./pubkey.h:635:26: warning: defaulted move assignment operator of 'InvertibleRSAFunction' will move assign virtual base class 'CryptoMaterial' multiple times [-Wmultiple-move-vbase]
class CRYPTOPP_NO_VTABLE TF_ObjectImpl : public TF_ObjectImplBase<BASE, SCHEME_OPTIONS, KEY_CLASS>
                         ^
./rsa.h:57:44: note: 'CryptoMaterial' is a virtual base class of base class 'CryptoPP::RSAFunction' declared here
class CRYPTOPP_DLL InvertibleRSAFunction : public RSAFunction, public TrapdoorFunctionInverse, public PKCS8PrivateKey
                                           ^~~~~~~~~~~~~~~~~~
./rsa.h:57:96: note: 'CryptoMaterial' is a virtual base class of base class 'CryptoPP::PKCS8PrivateKey' declared here
class CRYPTOPP_DLL InvertibleRSAFunction : public RSAFunction, public TrapdoorFunctionInverse, public PKCS8PrivateKey
                                                                                               ^
1 warning generated.

My apologies for not reducing it. I'm not sure how to reduce it and capture the essence of the warning/complaint.

like image 821
jww Avatar asked Dec 18 '22 21:12

jww


2 Answers

The warning seems self-explanatory to me, it's telling you that move-assigning the derived type will result in move-assigning the base twice.

Reducing it is trivial, just create an inheritance hierarchy using a virtual base and two paths to get to it:

#include <stdio.h>

struct V {
    V& operator=(V&&) { puts("moved"); return *this; }
};

struct A : virtual V { };

struct B : virtual V { };

struct C : A, B { };

int main() {
    C c;
    c = C{};
}

This will print "moved" twice, because the implicit move assignment operators for each of A, B and C will do a memberwise assignment, which means that both A::operator=(A&&) and B::operator=(B&&) will assign the base class. As Alan says, this is a valid implementation of the standard. (The standard specifies that on construction only the most-derived type will construct the virtual base, but it doesn't have the same requirement for assignment).

This isn't specific to move assignment, changing the base class to only support copy assignment and not move assignment will print "copied" twice:

struct V {
    V& operator=(const V&) { puts("copied"); return *this; }
};

This happens for exactly the same reason, both of A::operator=(A&&) and B::operator=(B&&) will assign the base class. The compiler doesn't warn for this case, because doing copy assignment twice is (probably) just suboptimal, not wrong. For move-assignment it might lose data.

If your virtual base doesn't actually have any data that needs to be copied or moved, or only has data members that are trivially copyable, then making it only support copying not moving will suppress the warning:

struct V {
    V& operator=(const V&) = default;
};

This copy assignment operator will still get called twice, but since it doesn't do anything there's no problem. Doing nothing twice is still nothing.

(GCC seems a bit smarter than Clang here, it doesn't warn about the virtual base's move assignment operator being called twice if it's trivial, because a trivial move is equivalent to a copy and so is less likely to be a problem).

If the virtual base does have data that needs to be copied on assignment then making it do a copy not a move might still be a good choice, but it depends what the type is and does. You might need to define copy and move assignment explicitly at every level of the hierarchy. Virtual bases are tricky and hard to use correctly, especially in the face of copying or moving. Treating types with virtual bases as value types that can be copied and moved easily might be a design error.

The iostreams hierarchy uses virtual bases, but is done carefully and correctly. The iostream types are non-copyable, only movable, and the derived types define move assignment explicitly to ensure the basic_ios<> base class only gets updated once. Specifically, basic_iostream::operator=(basic_iostream&&) only operates on the basic_istream base, not the basic_ostream one. The equivalent for the example above would be:

struct C : A, B {
     C& operator=(C&& c) {
         static_cast<A&>(*this) = static_cast<A&&>(c);
         return *this;
     }
};

Iostreams were not copyable at all until C++11 when rvalue references and move semantics made it possible to do with useful semantics. If your class has always been copyable in C++03 it might already have been a questionable design that should have had been non-copyable, or have carefully written copy operations not implicitly-defined ones.

In short, any time you have virtual bases you need to think very carefully about how construction, assignment and destruction work, and whether copying and assignment even make sense for the type.

like image 128
Jonathan Wakely Avatar answered Dec 24 '22 02:12

Jonathan Wakely


The standard allows implementations to choose a simple but sometimes broken way to handle memberwise assignment in the presence of virtual bases.

http://en.cppreference.com/w/cpp/language/move_assignment:

As with copy assignment, it is unspecified whether virtual base class subobjects that are accessible through more than one path in the inheritance lattice, are assigned more than once by the implicitly-defined move assignment operator.

This is particularly nasty for move assignment, since it may mean assigning from an already moved-from member.

like image 37
Alan Stokes Avatar answered Dec 24 '22 02:12

Alan Stokes