Should std::variant be nothrow destructible when its alternative has potentially throwing destructor?
Asked Answered
D

2

8

TL;DR: see compilers disagree on code by Godbolt link: https://godbolt.org/z/f7G6PTEsh

Should std::variant be nothrow destructible when its alternative has potentially throwing destructor? Clang and GCC seem to disagree on it (probably different standard library implementations on Godbolt?), MSVC thinks it should. cppreference says variant's destructor does not have noexcept specifications, and in that case destructor should be unconditionally noexcept unless it has potentially throwing members or bases (https://en.cppreference.com/w/cpp/language/noexcept_spec), which is not specified for variant (duh!).

The reason I'm asking is not to mess with throwing stuff out of destructors, but to gradually refactor weird legacy stuff that is minimized to the following snippet ("exception specification of overriding function is more lax than the base version"), and also related bug on dated compiler that we can't update yet.

#include <type_traits>
#include <variant>

struct S
{
    ~S() noexcept(false);
};

static_assert(std::is_nothrow_destructible_v<std::variant<S>>); // assertion failed with clang

struct Y
{
    virtual ~Y() {}
};

struct Z: public Y
{
    virtual ~Z() {}

    // clang error because of this member: 
    // exception specification of overriding function is more lax than the base version
    std::variant<int, S> member;
}
Discipline answered 10/6 at 13:56 Comment(3)
From std::variant "...the types that may be stored in this variant. All types must meet the Destructible requirements..." and from Destructible "...u.~T() All resources owned by u are reclaimed, no exceptions are thrown. ..." So an alternative type that can throw on destruction breaks the pre-conditions.Biological
TBH I believe STL and related stuff do not mix with throwing destructors at all, I just sometimes totally forget about it :).Discipline
Per answers to this question, I am now convinced that if a type's destructor is noexcept(false), it may still never actually throw, and then it is a valid alternative type for a variant.Discipline
E
4

GCC is wrong. According to [res.on.exception.handling]/3:

Destructor operations defined in the C++ standard library shall not throw exceptions. Every destructor in the C++ standard library shall behave as if it had a non-throwing exception specification.

How the destructor of std::variant is declared is beside the point; the library implementor is responsible for ensuring that the destructor behaves as if it were noexcept, wherever the presence or absence of such noexceptness can be detected.

Note that types used to instantiate standard library templates are not allowed to throw exceptions ([res.on.functions]/2.4) but that's not necessarily relevant to your example. The rule doesn't say that your S type must have a noexcept destructor. It just says that it's not allowed to actually throw. Violating this rule leads to UB, only if an execution that results in an exception escaping the destructor actually happens.

Emulsion answered 11/6 at 0:25 Comment(5)
If LWG3229 is resolved as proposed, then GCC complies because std::variant does not have a declared destructor. The wording is defective.Mikelmikell
It's also not 100% clear to me whether "behave as if it had a non-throwing exception specification" means that noexcept(t.~T()) is required to be true. After all, such a test in the user's code is not the behavior of the destructor itself.Mikelmikell
@JanSchultke I don't see what else it could possibly mean. If it simply referred to what happens inside the function, then it would be redundant, since the previous sentence already said that standard library functions can't throw exceptions.Emulsion
It wouldn't be redundant because it would further constrain the behavior to imply that std::terminate should be used when a user-thrown exception reaches the destructor of a standard library type, whereas saying nothing could imply any possible way of ensuring that it throws nothing. For what it's worth, it's more plausible to me that the noexcept test is intended to pass, just not worded in a way that makes me 100% certain.Mikelmikell
@JanSchultke I don't think it would do that, because that wouldn't happen unless a destructor or deallocation function throws, which results in UB, so you don't get the std::terminate guarantee anyway.Emulsion
M
6

std::variant doesn't work with throwing destructors

Firstly, this code could have undefined behavior because all variant alternatives must meet the Cpp17Destructible requirement ([variant.variant.general] p2), which mandates ([tab:cpp17.destructible]) for the expression u.~T():

All resources owned by u are reclaimed, no exception is propagated.

If ~S() threw an exception, the behavior would be undefined, although you can in principle have a noexcept(false) destructor with std::variant as long as it never throws.

I don't believe that the result of static_assert(std::is_nothrow_destructible_v<std::variant<S>>); is specified; the standard simply defines std::variant::~variant() as:

constexpr ~variant();

... which is noexcept depending on whether data members have a throwing destructor (see below for more). Data members are an implementation detail.

But why do compilers disagree?

However, this also appears to be a GCC bug because [except.spec] p8 states:

The exception specification for an implicitly-declared destructor, or a destructor without a noexcept-specifier, is potentially-throwing if and only if any of the destructors for any of its potentially constructed subobjects has a potentially-throwing exception specification or the destructor is virtual and the destructor of any virtual base class has a potentially-throwing exception specification.

In libstdc++, the destructor is explicitly defaulted (<variant> line 1512):

_GLIBCXX20_CONSTEXPR ~variant() = default;

In libstdc++, the actual member is stored within an _Uninitialized subobject, as

union {
    _Empty_byte _M_empty;
    _Type _M_storage;
};
// ...
~_Uninitialized() {}

A way to reproduce this situation is:

#include <type_traits>

struct D {
    ~D() noexcept(false);
};

struct B {
    union {
        char c;
        D d;
    };

    ~B() {}
};

static_assert(std::is_nothrow_destructible_v<B>);

The assertion passes only for GCC, and fails for other compilers (https://godbolt.org/z/1doaTrxsG).

Since S has a potentially throwing subobject of type D (indirectly), S should not be nothrow-destructible.

This is a known compiler bug; see GCC Bug #115222.

Mikelmikell answered 10/6 at 16:19 Comment(5)
But this is defective, right? Since union doesn't automatically call the member destructor, it shouldn't affect noexceptness of the enclosing class destructor.Westfahl
@Westfahl it says "if any of its potentially constructed subobjects" so I believe the intent is very clear; d is potentially constructed. You can still mark the destructor noexcept explicitly. Also note that the destructor would be deleted if it was = default; or implicitly-declared.Mikelmikell
It being deleted is alright. It's just weird that something that's not automatically executed (destroying union member) still affects the noexceptness.Westfahl
This would only be ill-formed if the destructor of S actually throws. noexcept(false) destructors are Cpp17Destructible as long as an exception doesn't escape the destructor callCompartmentalize
@Compartmentalize you're right, thanks. I've updated the question accordingly.Mikelmikell
E
4

GCC is wrong. According to [res.on.exception.handling]/3:

Destructor operations defined in the C++ standard library shall not throw exceptions. Every destructor in the C++ standard library shall behave as if it had a non-throwing exception specification.

How the destructor of std::variant is declared is beside the point; the library implementor is responsible for ensuring that the destructor behaves as if it were noexcept, wherever the presence or absence of such noexceptness can be detected.

Note that types used to instantiate standard library templates are not allowed to throw exceptions ([res.on.functions]/2.4) but that's not necessarily relevant to your example. The rule doesn't say that your S type must have a noexcept destructor. It just says that it's not allowed to actually throw. Violating this rule leads to UB, only if an execution that results in an exception escaping the destructor actually happens.

Emulsion answered 11/6 at 0:25 Comment(5)
If LWG3229 is resolved as proposed, then GCC complies because std::variant does not have a declared destructor. The wording is defective.Mikelmikell
It's also not 100% clear to me whether "behave as if it had a non-throwing exception specification" means that noexcept(t.~T()) is required to be true. After all, such a test in the user's code is not the behavior of the destructor itself.Mikelmikell
@JanSchultke I don't see what else it could possibly mean. If it simply referred to what happens inside the function, then it would be redundant, since the previous sentence already said that standard library functions can't throw exceptions.Emulsion
It wouldn't be redundant because it would further constrain the behavior to imply that std::terminate should be used when a user-thrown exception reaches the destructor of a standard library type, whereas saying nothing could imply any possible way of ensuring that it throws nothing. For what it's worth, it's more plausible to me that the noexcept test is intended to pass, just not worded in a way that makes me 100% certain.Mikelmikell
@JanSchultke I don't think it would do that, because that wouldn't happen unless a destructor or deallocation function throws, which results in UB, so you don't get the std::terminate guarantee anyway.Emulsion

© 2022 - 2024 — McMap. All rights reserved.