[libcxx][PATCH] N3672 - optional

[Howard Hinnant]

Thanks Richard.

On Aug 29, 2013, at 7:31 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> "::new(&__null_state_) value_type(...)" seems a bit strange (and I'm concerned that it would probably lead to undefined behavior, if we actually had comprehensible rules for unions...). Maybe "::new(addressof(__val_)) value_type(...)"?

Ok, done.

> 
> Do you need the (in_place_t, initializer_list, Args...) constructor in __optional_storage? The (in_place_t, Args...) constructor looks like it would appropriately handle initializer_list construction, since the optional constructor will have already selected the right argument type.

Agreed.  Confirmed all tests pass with these removed.  Also confirmed that this constructor is being tested.  Removing (in_place_t, initializer_list, Args...) from optional causes optional.object.ctor/initializer_list.pass.cpp to fail.

Removed these constructors from __optional_storage.

> 
> Oh god, there's overload sets for operator->(), operator*(), value(), and value_or(), where one is constexpr and the other isn't. That's awful (and unnecessary in C++14). =( Is there an LWG issue on that?

I see no LWG issue in this area.

Note that the non-const versions differ in return value.  I.e. unlike smart pointers, optional is doing "deep const".  Whether or not that is the right design, I have not investigated.  However my first guess is that the spec is correct here.

> 
> (You can also use __builtin_addressof in recent versions of Clang to make these constexpr whether or not there is an overloaded operator&, but I don't think the current standard actually allows you to do that...)
> 
> __has_operator_addressof_imp is not robust against operator&() returning a type with an overloaded operator,(), and will trigger the instantiation of the return type of operator&() (which might fail) when looking for such an operator,(). I suggest following the pattern in __is_constructible_test:
> 
> static auto __test(_Up* __u)
>   -> typename __select_2nd<decltype(__u->operator&()), true_type>::type;

Thanks, fixed and confirmed with test.

> 
> Looks like __is_assignable_test has the same bug, FWIW.

Thanks, fixed and confirmed with test.

Updated patch enclosed.

Howard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: optional.patch
Type: application/octet-stream
Size: 142470 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130831/b3ce7517/attachment.obj>