I asked a question about move constructors for which I haven't accepted an answer yet because I'm feeling more confused about certain aspects of the question even as I'm starting to get a grip on others. In particular, I've found a surprising case in which both g++ and clang++ generate incorrect move-constructors.
delete
in its destructor. Does the Qt library (version 5.4) do this?Consider the following code:
class NoMove
{
public:
~NoMove() {}
};
int main()
{
std::cout << "NoMove move-constructible? " <<
std::is_move_constructible<NoMove>::value << std::endl;
}
Compiled with both g++
4.9.2 and clang++
3.5.1, this code prints:
NoMove move-constructible? 1
...But since NoMove
has an explicitly defined destructor, I would expect that neither a move constructor nor a copy constructor should be auto-generated. Note that the unexpected constructor generation is not due to the fact that the destructor is trivial; I get the same behavior when the destructor delete[]
s an array (!!), and I am even able to compile code that requires a valid move constructor (!!!!!). (See example below.) What's going on here? Is it legal to auto-generate a move constructor here, and if so, why?
It appears that providing safe move constructors when delete
is involved is fairly simple, but I just want to make sure I understand: when a class contains a pointer member and owns the underlying data, is there any case in which it wouldn't be correct and sufficient for the move constructor to invalidate the RHS pointer after setting the destination pointer to the old value?
Consider the following example, which is similar to the NoMove
example above and is based on my original question:
class DataType
{
public:
DataType()
{
val = new int[35];
}
~DataType()
{
delete[] val;
}
private:
int* val;
};
class Marshaller
{
public:
Marshaller()=default;
DataType toDataType() &&
{
return std::move(data);
}
private:
DataType data;
};
void DoMarshalling()
{
Marshaller marshaller;
// ... do some marshalling...
DataType marshalled_data{std::move(marshaller).toDataType()};
}
This compiles just fine--showing that, yes, DataType
has an auto-generated move constructor. And of course, when run, it causes a double-deletion error.
Now, this would be okay, if the auto-generated move constructor invalidated the RHS pointer. So, if it's okay to auto-generate a move constructor here, why isn't that done safely? The move constructor that makes this work is simply:
DataType(DataType&& rhs) :
val{rhs.val}
{
rhs.val = nullptr;
}
(Right? Am I missing anything? Should it perhaps be val{std::move(rhs.val)}
?)
This seems like it would be a perfectly safe function to auto-generate; the compiler knows that rhs
is an r-value because the function prototype says so, and therefore it's entirely acceptable to modify it. So even if DataType
's destructor didn't delete[] val
, it seems like there wouldn't be any reason not to invalidate rhs
in the auto-generated version, except, I suppose, for the fact that this leads to a trivial performance hit.
So if the compiler is auto-generating this method--which, again, it shouldn't, especially since we can just as easily get this exact behavior from standard library code using unique_ptr
-- why is it auto-generating it incorrectly?
QByteArray
in Qt 5.4)Finally, a (hopefully) easy question: do Qt 5.4's heap-allocating classes such as QByteArray
(which is what I'm actually using as the DataType
in my original question) have correctly implemented move constructors, invalidating any moved-from owning pointer(s)?
I wouldn't even bother to ask, because Qt seems pretty solid and I haven't seen any double-deletion errors yet, but given that I was taken off guard by these incorrect compiler-generated move constructors, I'm concerned that it's quite easy to end up with incorrect move constructors in an otherwise-well-implemented library.
Relatedly, what about Qt libraries written before C++11
that don't have explicit move-constructors? If I can accidentally coerce an auto-generated move constructor that behaves erroneously in this case, does anyone know if compiling, say, Qt 3 with a C++11-compliant compiler causes undefined destruction behavior in use-cases like this?
If a copy constructor, copy-assignment operator, move constructor, move-assignment operator, or destructor is explicitly declared, then: No move constructor is automatically generated. No move-assignment operator is automatically generated.
Implicitly-defined move constructor For non-union class types (class and struct), the move constructor performs full member-wise move of the object's bases and non-static members, in their initialization order, using direct initialization with an xvalue argument.
A move constructor enables the resources owned by an rvalue object to be moved into an lvalue without copying.
The problem is that you are confusing is_move_constructible
and "has a move constructor". is_move_constructible<T>
doesn't test whether T has a move constructor. It tests whether T
can be constructed from an rvalue of type T
. And const T&
can bind to a T
rvalue.
What you are seeing is the autogenerated copy constructor T(const T&)
doing its work - and failing miserably.
I would expect that neither a move constructor nor a copy constructor should be auto-generated.
Your link talks about the move constructor. It doesn't talk about the copy constructor, which is always implicitly declared if you don't declare it.
Now, if you declared a move operation, the implicitly declared copy constructor would be defined as deleted, but you didn't do that, so it's defined as defaulted and performs a memberwise copy. [class.copy]/p7:
If the class definition does not explicitly declare a copy constructor, one is declared implicitly. If the class definition declares a move constructor or move assignment operator, the implicitly declared copy constructor is defined as deleted; otherwise, it is defined as defaulted (8.4). The latter case is deprecated if the class has a user-declared copy assignment operator or a user-declared destructor.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With