<variant>: std::variant template param order determines if destructor is constant expression

[Veeloxfire]

Describe the bug

The order of the template parameters in variant influences if the destructor is a constant expression.
I think it might be related to a compiler behaviour/bug with constexpr destructors being weird with unions.

It seems that it requires the very first element to have a constexpr destructor in order for the whole object to have a constexpr destructor.
From playing around it also seems like you're using the only case where this even works at all on msvc: recursive templates where the outer-most case is the first union field and has a constexpr destructor.

Command-line test case

https://godbolt.org/z/h5b766seM

#include <variant>

struct Trivial {};

struct NonTrivial {
    NonTrivial() {}
    ~NonTrivial() {}
};

// Trivial first
constexpr std::variant<Trivial, NonTrivial> v0{Trivial{}};// no error
// Nontrivial first
constexpr std::variant<NonTrivial, Trivial> v1{Trivial{}};// error

Expected behavior

I would expect it to either error on both, or error on neither

STL version

It becomes an issue since VS16.11 up until the latest on godbolt (idk specifically which stl version that is sorry)
Before that both are errors so its not a problem I guess

Additional context

There are 4 potential solutions I could think of:

1. Say the current behaviour is intended (strange but potentially only good option)
2. Make this invalid in both cases (don't do this please)
3. Somehow fix the msvc compiler to make the destructor constexpr-ness not be so weird
4. Detect if there is a constexpr destructable member in some outer template, and reorder the template for storage so that the constexpr one is always first. But I don't know if this would be possible and still remain standard compliant and not break abi (I suspect it would break both)

[frederick-vs-ja]

This is probably a bug MSVC as the example is correctly accepted by Clang + MSVC STL (and Clang + libc++, Clang + libstdc++, GCC + libstc++).

[Veeloxfire]

I was going to say the same (and I have actually reported this compiler bug in october 2023 but its still unfixed). But I just came up with a way to fix it?
The problem making sure the outermost variant union is constexpr. The errors originates from _Vector_storage_ so my thought was that what if you artificially inserted an extra element in front of the variant list

Splitting _Variant_storage_recurse out of _Variant_storage and making _Variant_storage the external api, that adds in something like _Placeholder_ty_variant_start as the first element so that the outermost is always trivial

Then specializations for when _Placeholder_ty_variant_start is the first type that don't actually do anything other than forward constructor parameters

So far all I tried was to get this to compile, which it does! (the only other change is inside get you also have to add some extra ._Variant_Internal)
But idk if this passes tests or is even an okay change

what are your thoughts on a fix like this?

template <bool _TrivialDestruction, class... _Types>
class _Variant_storage_ {};

struct _Placeholder_ty_variant_start {};

template <class... _Types>
using _Variant_storage = _Variant_storage_<conjunction_v<is_trivially_destructible<_Types>...>, _Placeholder_ty_variant_start, _Types...>;

template <class... _Types>
using _Variant_storage_recurse = _Variant_storage_<conjunction_v<is_trivially_destructible<_Types>...>, _Types...>;

template <class _First, class... _Rest>
class _Variant_storage_<true, _First, _Rest...> { // Storage for variant alternatives (trivially destructible case)
    // ....

    union {
        remove_cv_t<_First> _Head;
        _Variant_storage_recurse<_Rest...> _Tail;
    };

   // ....
};

template <class _First, class... _Rest>
class _Variant_storage_<false, _First, _Rest...> { // Storage for variant alternatives (non-trivially destructible case)
    // ....
    
    union {
        remove_cv_t<_First> _Head;
        _Variant_storage_recurse<_Rest...> _Tail;
    };

     // ....
};


template <class... _Rest>
class _Variant_storage_<true, _Placeholder_ty_variant_start, _Rest...> { // Outermost storage for variant alternatives (trivially destructible case)
    // ....

    union {
        char _placeholder_Head;
        _Variant_storage_recurse<_Rest...> _Variant_Internal;
    };

    // ....
};

template <class... _Rest>
class _Variant_storage_<false, _Placeholder_ty_variant_start, _Rest...> { // Outermost storage for variant alternatives (non-trivially destructible case)
    // ....

    union {
        char _placeholder_Head;
        _Variant_storage_recurse<_Rest...> _Variant_Internal;
    };

     // ....
};

[StephanTLavavej]

I have actually reported this compiler bug in october 2023 but its still unfixed

What's the VS Developer Community bug number/link?

what are your thoughts on a fix like this?

That's a library workaround, not a true fix. The best solution is to get a compiler fix. We do add library workarounds from time to time, but we can't mess with data structure representations in any significant way because that would break ABI (binary compatibility).

[Veeloxfire]

Bug report link: https://developercommunity.microsoft.com/t/Constexpr-union-destructor-cannot-result/10486017
The bug report is slightly outdated because at the time I didn't know about any possible workarounds

That's a library workaround, not a true fix.

I too would prefer a compiler fix but I waited for a year and that didn't happen (which is fair there are more important bugs). Just thought I might try to think of a workaround in the meantime

we can't mess with data structure representations in any significant way

Also yes abi, I was unsure if change to a union representation with an additional empty member would break abi since in practice the object representation should end up the same (I thought I had mentioned that but apparently I forgot)

[StephanTLavavej]

Thanks, I've marked our internal mirror of that bug as "STL-requested" and noted the std::variant impact.