[Development] Out of line move constructors

[Volker Hilsheimer]

On 14 Sep 2020, at 20:11, Thiago Macieira <thiago.macieira at intel.com> wrote:
> 
> On Monday, 14 September 2020 03:37:00 PDT Giuseppe D'Angelo via Development 
> wrote:
>> Hi,
>> 
>> Many pimpled types in Qt lack a move constructor.
>> 
>> History: back in the day, when C++11 wasn't a requirement and binary
>> compatibility between C++98/C++11 was to be kept, we had to implement
>> C++11-only code (such as move operations) inline. This however isn't
>> possible for move constructors if the type is implemented using pimpl
>> (with or without refcounting).
> 
> The problem is not a pimpl d-pointer, it's only when said pointer is held by a 
> QScopedPointer or similar. The reason for that is the code generation for the 
> move constructor must handle the case where an exception is thrown, so it must 
> generate a call to the destructor of the smart pointer pointer. Since the 
> payload type is only forward declared, the compilation fails.
> 
> This has nothing to do with whether an exception *can* be thrown in that 
> context or not or whether the compiler will generate a call to the destructor 
> anywhere. The compiler simply has to pretend that it could.
> 
> See https://gcc.godbolt.org/z/KsjWq7.
> 
>> Coming 5.9, C++11 became a requirement. Still we didn't add move
>> constructors for pimpled types; the rationale was that we wanted move
>> operations to be inline. For refcounted types, the idea was to achieve
>> that (I think) by using something like QIntrusiveSharedPointer instead
>> of Q(E)SDP.
>> 
>> Anyhow: is this still the policy, or do we want to start adding out of
>> line move constructors?
> 
> There's a workaround for the problem above, which is to declare the 
> QScopedPointer's destructor as a forward-declaration. This allows compilation 
> to proceed and/or even make the destructor inline.
> 
> See https://gcc.godbolt.org/z/vGP6oW (difference is line 46).


Thanks for raising this Giuseppe, and for the clarification and workaround, Thiago.

I’ve tried to make this work for

https://codereview.qt-project.org/c/qt/qtbase/+/313610

but now I get ~QExplicitlySharedDataPointer<QPlatformPixmap> as an unresolved external, presumably because the unspecialized ~QExplicitlySharedDataPointer is inline.


Volker