mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

Behaviour when contained type has overload for unary operator & #11

Closed hammond closed 7 years ago

hammond commented 7 years ago

Hi!

First of all, thanks for implementing this, great job!

I noticed that variant<> would not compile when I used a type with overloaded operator& as an alternative. access::get_if gave the following error:

'auto' in return type deduced as 'thing' here but deduced as 'wrapper' in earlier return statement.

wrapper<thing>::operator&, in my case, is returning thing *.

Changing get_if like this makes that part work:

 constexpr auto *get_if(Variant *v) {
   using V = decay_t<Variant>;
   static_assert(I < experimental::tuple_size<V>::value, "");
   using T = experimental::tuple_element_t<I, V>;
   assert(v);
   if (!holds_alternative<I>(*v)) {
     using R = add_pointer_t<lib::qualify_as_t<T, Variant>>;
     return static_cast<R>(nullptr);
   }  // if
-  return &unsafe::get<I>(*v);
+  return std::addressof(unsafe::get<I>(*v));
 }

I also had to change the UNION_IMPL macro like this to avoid some nasty crashes:

     template <typename... Args>                                                                              \
     void construct(lib::size_constant<0>, Args &&... args) {                                                 \
-      ::new (&head_) head(forward<Args>(args)...);                                                           \
+      ::new (std::addressof(head_)) head(forward<Args>(args)...);                                            \
     }                                                                                                        \

I am not sure about the changes above, but it seems to be what the standard wants: http://en.cppreference.com/w/cpp/utility/variant/get_if

It seems like std::addressof() does not become constexpr until C++17 though...

Cheers, Erik

mpark commented 7 years ago

Hi Erik! Sorry for the delayed response. I just finished implementing the latest specification of std::variant, and I've introduced cpp17::addressof which tries to be constexpr as possible.