[Development] QVariant and types with throwing dtors

[Volker Hilsheimer]

> On 9 Aug 2024, at 17:52, Thiago Macieira <thiago.macieira at intel.com> wrote:
> 
> On Friday 9 August 2024 01:46:02 GMT-7 Fabian Kosmale via Development wrote:
>> 1. Don't do anything; that's the behavior we have since at least Qt 5.
>> If the dtor doesn't actually throw, everything is fine; if it does
>> throw, we're calling std::terminate. Might lead to unexpected results,
>> but doing nothing obviously doesn't break any existing code. It is
>> however inconsistent with other types.
>> 2. Warn, but still accept such types [2].  I've implemented that in
>> https://codereview.qt-project.org/c/qt/qtbase/+/580560. Anyone one
>> stores a type with a throwing dtor would get a warning, but everything
>> still compiles. The warning can however be considered spurious if the
>> author knows that given their usage, the type would never throw.
>> 3. Reject such types at compile time. That would be consistent with
>> other types, but might break existing code, even code that works
>> perfectly fine because it can never actually throw.
>> 
>> I'm leaning towards either 1 (nobody ever reported a bug about QVariant
>> calling std::terminate so far), or 2 ( more consistent with our policy
>> for containers, highlights that such usage is potentially dangerous).
> 
> I think we're fine just leaving it as-is, case 1. As you said, no one has 
> reported a problem and I doubt they ever will. We cannot make QVariant 
> destructor throwing, because that would in turn make QVariantList impossible. 
> For that matter, std::any's destructor is also noexcept.
> 
> One problem with a warning or rejecting (cases 2 and 3) is that we have to 
> provide a way for people to fix it. If their type does indeed need a throwing 
> destructor, then they can't make it noexcept. They can't wrap it in something 
> else (is-a or has-a) because this is QVariant and the actual type matters. We 
> can provide a macro to return to current state for code that knows what it is 
> doing, but that increases our maintenance burden for something that has never 
> been a problem.
> 
> You also can't completely accomplish this at compile time, because you can 
> create a QVariant payload with just a metatype:
> 
>    QVariant v(QMetaType::fromType<ThrowingDestructor>());
> 
> As you're not proposing rejecting throwing-destructor-types from QMetaType, we 
> can't compile-time reject the above. We'd need to do it at runtime, at the 
> expense of another bit extracted into the QMTI, and impact everyone else's 
> runtime to check that bit and warn or reject. We'd probably not do the runtime 
> checking at all, because of the side effect and because the construction of the 
> user code as above could mean the developer never sees the problem until 
> releasing.
> 
> I don't think it's worth it.


Agree with Thiago here. The problem is a bit academic, and doesn’t seem large enough to justify the complexity required in making a bullet-proof solution.

Volker