<variant>: variant::operator=(T&&) accept wrong case

[hewillk]

From [variant#assign-13] (emphasis mine):

Effects:
If *this holds a Tj, assigns std​::​forward<T>(t) to the value contained in *this.
Otherwise, if is_­nothrow_­constructible_­v<Tj, T> || !is_­nothrow_­move_­constructible_­v<Tj> is true, equivalent to emplace<j>(std​::​forward<T>(t)).
Otherwise, equivalent to operator=(variant(std​::​forward<T>(t))).

For the last case, MSVC STL did not call operator=(variant(std​::​forward<T>(t))) but called _Emplace_valueless<_TargetIdx>(_STD move(_Temp)):

STL/stl/inc/variant

Lines 1068 to 1093 in 78ff461

	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>
	_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>) {
	// assign/emplace the alternative chosen by overload resolution of _Obj with f(_Types)...
	constexpr size_t _TargetIdx = _Variant_init_index<_Ty, _Types...>::value;
	if (index() == _TargetIdx) {
	auto& _Target = _Variant_raw_get<_TargetIdx>(_Storage());
	_Target = static_cast<_Ty&&>(_Obj);
	} else {
	using _TargetTy = _Variant_init_type<_Ty, _Types...>;
	if constexpr (_Variant_should_directly_construct_v<_TargetTy, _Ty>) {
	this->_Reset();
	_Emplace_valueless<_TargetIdx>(static_cast<_Ty&&>(_Obj));
	} else {
	_TargetTy _Temp(static_cast<_Ty&&>(_Obj));
	this->_Reset();
	_Emplace_valueless<_TargetIdx>(_STD move(_Temp));
	}
	}
	return *this;
	}

Consider (godbolt):

#include <variant>

struct A { 
  A() = default;
  A(A&&) = delete;
};

struct B { 
  B(A&&) {};
};

int main() {
  std::variant<A, B> v{};
  v = A{};
}

In this case, _Variant_should_directly_construct_v (that is, is_nothrow_constructible_v<B, A> || !is_nothrow_move_constructible_v<B>) is false, so we call operator=(variant&&), but since A's move constructor is deleted, we should fail.

Please note that MSVC v19.28 and v19.29 under /std:c++17 correctly rejected it, so I don't know where the problem is (it seems to be in 87152f4).

[CaseyCarter]

This is not the result of a bug, this is an intentional behavior change from C++20 (via WG21-P0608) that we do not implement in earlier language modes due to the potential for breakage. In C++20, [variant.assign]/11 determines the alternative type Tj that should be either assigned to or made active for a given argument type T:

Let Tj be a type that is determined as follows: build an imaginary function FUN(Ti) for each alternative type Ti for which Ti x[] = {std::forward(t)}; is well-formed for some invented variable x. The overload FUN(Tj) selected by overload resolution for the expression FUN(std::forward(t)) defines the alternative Tj which is the type of the contained value after assignment.

When T is A and t is an lvalue of type A, as in the sample program,

A x[] = { std::forward<A>(t) };

is ill-formed since A has a deleted move constructor, but

B x[] = { std::forward<A>(t) };

is well-formed. Consequently, the imaginary overload set only contains FUN(B), which is selected by overload resolution, and the sample program correctly changes the active alternative of v to a B initialized from A{}.

(I suspect libstdc++ hasn't implemented this C++20 change yet, but on the off chance it's a bug let's tag @jwakely.)

[jwakely]

P0608 was implemented for GCC 10.1, but is enabled unconditionally for C++17 and C++20.

[CaseyCarter]

P0608 was implemented for GCC 10.1, but is enabled unconditionally for C++17 and C++20.

I think you have a bug in your assignment operator, then. Interestingly, the converting constructor seems to do the right thing : https://godbolt.org/z/rxTGPzcx9.

[hewillk]

Consequently, the imaginary overload set only contains FUN(B), which is selected by overload resolution, and the sample program correctly changes the active alternative of v to a B initialized from A{}.

I don't understand.

Under your description, Tj is B and T is A, so we will initialize B with A{}, but isn't this behavior defined in [variant#assign-13] as using deleted variant::operator=(variant&&) for B's initialization? Just as libstdc++ complained:

gcc-trunk-20210830/include/c++/12.0.0/variant:1465:26: error: use of deleted function 'std::variant<_Types>& std::variant<_Types>::operator=(std::variant<_Types>&&) [with _Types = {test()::A, test()::B}]'

[jwakely]

https://cplusplus.github.io/LWG/issue3585 says that libstdc++ is correctly implementing what the standard requires, warts and all.

[CaseyCarter]

Thank you @hewilk for pointing out that the wording doesn't say what I want it to say despite my determined efforts to be unteachable, and thank you @jwakely for filing this LWG issue to keep variant's assignment operators consistent with its constructors.

[hewillk]

I think this issue was submitted by @brevzin, thank you.

[frederick-vs-ja]

This seems fixed since LWG-3585 has been voted into the Working Paper...

[CaseyCarter]

This seems fixed since LWG-3585 has been voted into the Working Paper...

Yes it is - this issue fell through the cracks. LWG-3585 standardized our implementation.