ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
675 stars 62 forks source link

checked<T,E> is not move assignable if T is not copy constructible #259

Closed DiegoBaldassarMilleuno closed 2 years ago

DiegoBaldassarMilleuno commented 2 years ago

See godbolt.org

DiegoBaldassarMilleuno commented 2 years ago

Proposed fix:

  template <class Base> struct value_storage_delete_copy_constructor : Base  // NOLINT
  {
    using Base::Base;
    using value_type = typename Base::value_type;
    using error_type = typename Base::error_type;
    value_storage_delete_copy_constructor() = default;
    value_storage_delete_copy_constructor(const value_storage_delete_copy_constructor &) = delete;
    value_storage_delete_copy_constructor(value_storage_delete_copy_constructor &&) = default;  // NOLINT
+     value_storage_delete_copy_constructor &operator=(const value_storage_delete_copy_constructor &o) = default;
+     value_storage_delete_copy_constructor &operator=(value_storage_delete_copy_constructor &&o) = default;  // NOLINT
  };

value_storage_delete_move_constructor may need some love too...

DiegoBaldassarMilleuno commented 2 years ago

Copy assignment is also problematic: godbolt.org

The following would seem to fix it:

  using value_storage_select_move_assignment =
  std::conditional_t<std::is_trivially_move_assignable<devoid<T>>::value && std::is_trivially_move_assignable<devoid<E>>::value,
                     value_storage_select_copy_constructor<T, E>,
                     std::conditional_t<std::is_move_assignable<devoid<T>>::value && std::is_move_assignable<devoid<E>>::value,
                                        value_storage_nontrivial_move_assignment<value_storage_select_copy_constructor<T, E>>,
-                                         value_storage_delete_copy_assignment<value_storage_select_copy_constructor<T, E>>>>;
+                                         value_storage_delete_move_assignment<value_storage_select_copy_constructor<T, E>>>>;
ned14 commented 2 years ago

Dear dear, I cannot believe this was never reported until now. Thanks for the BR!

DiegoBaldassarMilleuno commented 2 years ago

No problem!

ned14 commented 2 years ago

Fixed! Thanks for the BR!