3069. Move assigning variant's subobject corrupts data

Section: 22.6.3.4 [variant.assign] Status: New Submitter: Antony Polukhin Opened: 2018-02-20 Last modified: 2020-09-06

Priority: 3

View other active issues in [variant.assign].

View all other issues in [variant.assign].

View all issues with New status.

Discussion:

variant::emplace functions in 22.6.3.5 [variant.mod] destroy the currently contained value before initializing it to a new value. Assignments in 22.6.3.4 [variant.assign] are described in terms on emplace.

This leads to situation, when move/copy assigning subobject from variant into the same variant corrupts data:

#include <variant>
#include <memory>
#include <iostream>

using str_t = std::string;
using str_ptr_t = std::unique_ptr<str_t>;
using var_t = std::variant<str_t, str_ptr_t>;

int main() 
{
  var_t v = str_ptr_t{
    new str_t{"Long string that does not fit into SS buffer and forces dynamic allocation"}
  };

  // Any of the following lines corrupt the variant's content:
  v = *std::get<str_ptr_t>(v);
  //v = std::move(*std::get<str_ptr_t>(v));

  std::cout << std::get<str_t>(v) << std::endl; // SEGV - 'str_t' inside 'v' is invalid
}

Such behavior confuses users, especially those users who are used to boost::variant's behavior. Consider making variant assignments safer by defining them close to copy-and-swap.

[2018-06-18 after reflector discussion]

Priority set to 3; Antony volunteered to write a paper for Rapperswil.

Proposed resolution:

This wording is relative to N4727.

1. Change 22.6.3.4 [variant.assign] as indicated:

   variant& operator=(const variant& rhs);
   

   -1- Let j be rhs.index().

   -2- Effects:

   1. (2.1) — If neither *this nor rhs holds a value, there is no effect.

   2. (2.2) — Otherwise, if *this holds a value but rhs does not, destroys the value contained in *this and sets *this to not hold a value.

   3. (2.3) — Otherwise, if index() == j, assigns the value contained in rhs to the value contained in *this.

   4. (2.4) — Otherwise, if either is_nothrow_copy_constructible_v<Tj> is true or is_nothrow_move_constructible_v<Tj> is false, equivalent to emplace<j>(get<j>(rhs)).

   5. (2.5) — Otherwise, equivalent to emplace<j>(Tj{get<j>(rhs)})operator=(variant(rhs)).

   […]

   variant& operator=(variant&& rhs) noexcept(see below);
   

   -6- Let j be rhs.index().

   -7- Effects:

   1. (7.1) — If neither *this nor rhs holds a value, there is no effect.

   2. (7.2) — Otherwise, if *this holds a value but rhs does not, destroys the value contained in *this and sets *this to not hold a value.

   3. (7.3) — Otherwise, if index() == j, assigns get<j>(std::move(rhs)) to the value contained in *this.

   4. (7.4) — Otherwise, equivalent to emplace<j>(Tj{get<j>(std::move(rhs))}).

   […]

   -10- Let Tj be a type that is determined as follows: build an imaginary function FUN(Ti) for each alternative type Ti. The overload FUN(Tj) selected by overload resolution for the expression FUN(std::forward<T>(t)) defines the alternative Tj which is the type of the contained value after assignment.

   -11- Effects:

   1. (11.1) — If *this holds a Tj, assigns std::forward<T>(t) to the value contained in *this.

   2. (11.2) — Otherwise, if is_nothrow_constructible_v<Tj, T> || !is_nothrow_move_constructible_v<Tj> is true, equivalent to emplace<j>(std::forward<T>(t)).

   3. (11.3) — Otherwise, equivalent to emplace<j>(Tj{std::forward<T>(t)})operator=(variant(std::forward<T>(t))).

   […]