Skip to content

Commit

Permalink
Short-circuit some variant constraints
Browse files Browse the repository at this point in the history
`variant`'s converting constructor and assignment operator templates are constrained to reject arguments of the `variant`'s type. In such a case, the templates instantiated to check the constructibility constraint might be ill-formed outside the immediate context of template instantiation causing a hard error. We should split the constraints into multiple `enable_if_t`s to enable short-circuiting of later constraints when the earlier constraints fail.

Fixes microsoft#4959.
  • Loading branch information
CaseyCarter committed Sep 19, 2024
1 parent a26f4ad commit fe6f1dd
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
19 changes: 9 additions & 10 deletions stl/inc/variant
Original file line number Diff line number Diff line change
Expand Up @@ -934,12 +934,11 @@ public:
: _Mybase(in_place_index<0>) {} // value-initialize alternative 0

template <class _Ty,
enable_if_t<sizeof...(_Types) != 0 //
&& !is_same_v<_Remove_cvref_t<_Ty>, variant> //
&& !_Is_specialization_v<_Remove_cvref_t<_Ty>, in_place_type_t> //
&& !_Is_in_place_index_specialization<_Remove_cvref_t<_Ty>> //
&& is_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>, //
int> = 0>
enable_if_t<sizeof...(_Types) != 0 && !is_same_v<_Remove_cvref_t<_Ty>, variant>
&& !_Is_specialization_v<_Remove_cvref_t<_Ty>, in_place_type_t>
&& !_Is_in_place_index_specialization<_Remove_cvref_t<_Ty>>,
int> = 0,
enable_if_t<is_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>, int> = 0>
constexpr variant(_Ty&& _Obj) noexcept(is_nothrow_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>)
: _Mybase(in_place_index<_Variant_init_index<_Ty, _Types...>::value>, static_cast<_Ty&&>(_Obj)) {
// initialize to the type selected by passing _Obj to the overload set f(Types)...
Expand Down Expand Up @@ -976,10 +975,10 @@ public:
// initialize alternative _Idx from _Ilist and _Args...
}

template <class _Ty, enable_if_t<!is_same_v<_Remove_cvref_t<_Ty>, variant>
&& is_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>
&& is_assignable_v<_Variant_init_type<_Ty, _Types...>&, _Ty>,
int> = 0>
template <class _Ty, enable_if_t<!is_same_v<_Remove_cvref_t<_Ty>, variant>, int> = 0,
enable_if_t<is_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>
&& is_assignable_v<_Variant_init_type<_Ty, _Types...>&, _Ty>,
int> = 0>
_CONSTEXPR20 variant& operator=(_Ty&& _Obj)
noexcept(is_nothrow_assignable_v<_Variant_init_type<_Ty, _Types...>&, _Ty>
&& is_nothrow_constructible_v<_Variant_init_type<_Ty, _Types...>, _Ty>) {
Expand Down
28 changes: 28 additions & 0 deletions tests/std/tests/P0088R3_variant_msvc/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <functional>
#include <limits>
#include <memory>
#include <optional>
#include <string>
#include <type_traits>
#include <utility>
Expand Down Expand Up @@ -909,6 +910,33 @@ namespace msvc {
}
}
} // namespace assign_cv

namespace gh4959 {
// Test GH-4959 "P0608R3 breaks flang build with Clang"
// Constraints on variant's converting constructor and assignment operator templates reject arguments of the
// variant's type, but did not short-circuit to avoid evaluating the constructibility constraint. For this
// program, the constructibility constraint is ill-formed outside the immediate context when determing if
// variant<optional<GenericSpec>> can be initialized from an rvalue of the same type.

template <typename... RVREF>
using NoLvalue = std::enable_if_t<(... && !std::is_lvalue_reference_v<RVREF>)>;

struct Name {};

struct GenericSpec {
template <typename A, typename = NoLvalue<A>>
GenericSpec(A&& x) : u(std::move(x)) {}
GenericSpec(GenericSpec&&) = default;
std::variant<Name> u;
};

struct InterfaceStmt {
template <typename A, typename = NoLvalue<A>>
InterfaceStmt(A&& x) : u(std::move(x)) {}
InterfaceStmt(InterfaceStmt&&) = default;
std::variant<std::optional<GenericSpec>> u;
};
} // namespace gh4959
} // namespace msvc

int main() {
Expand Down

0 comments on commit fe6f1dd

Please sign in to comment.