optional(Expr&&) is insufficiently constrained

[akrzemi1]

Hi,

A user reported a bug (VSO#191303/Connect#2351203, see [1]) using Boost.Optional with MSVC 2015 Update 2. The original test case involved a minor bug in VC's tuple, which I'll look into fixing in the future (specifically, N-tuples recursively construct (N - 1)-tuples, in a way that's visible to overload resolution). However, there is also a bug in boost::optional.

Here's a reduced test case, which contains the relevant parts of boost::optional, and doesn't involve that bug in VC's tuple:

C:\Temp>type meow.cpp #include <tuple> #include <type_traits> #include <utility> using namespace std;

struct none_t { };

template <typename T> struct optional {

T thing;

optional(none_t)

: thing() { }

template <typename Expr,

typename = typename enable_if<

!is_same<typename decay<Expr>::type, none_t>::value

#ifdef FIX

&& is_constructible<T, Expr>::value

#endif

::type>

explicit optional(Expr&& expr)

: thing(forward<Expr>(expr)) { }

};

int main() {

tuple<none_t> one{};

tuple<optional<double>> two(one);

}

C:\Temp>cl /EHsc /nologo /W4 meow.cpp meow.cpp meow.cpp(22): error C2440: 'initializing': cannot convert from 'std::tuple<none_t>' to 'double' [...context...]

C:\Temp>cl /EHsc /nologo /W4 /DFIX meow.cpp meow.cpp

C:\Temp>

This is happening because I implemented N4387 (see [2]) and the Proposed Resolution for LWG 2549 (see [3]). The PR is required to implement N4387 safely.

The direct-init of two from one is attempting to select tuple's converting copy ctor (which N4387 marks as implicit, because optional's constructor from none_t is implicit). However, LWG 2549 additionally constrains the converting copy ctor. In English, when the source is tuple<A> and the destination is tuple<B>, LWG 2549 determines whether A is constructible from tuple<B>. If it IS, then the converting copy ctor vanishes, so that tuple's perfect forwarding ctor (tuple(UTypes&&...)) can construct A from tuple<B>. Alternatively, if A ISN'T constructible from tuple<B>, then the perfect forwarding ctor SFINAEs away while the converting copy ctor is permitted to participate, so A is constructed from the B within the tuple<B>.

The boost::optional bug is that optional(Expr&&) isn't sufficiently constrained, so it appears to be constructible (via is_constructible) from tuple<none_t>, even though the definition will explode. So LWG 2549 makes tuple's converting copy ctor vanish, and we end up attempting to construct optional<double> from tuple<none_t> which is doomed.

This is NOT a problem with LWG 2549's Proposed Resolution, NOR is it a problem in VC 2015 Update 2's tuple (which has correctly implemented this, at least for the 1-tuple case, as previously mentioned).

The fix that should be applied to boost::optional is to constrain optional(Expr&&) based on whether its definition will compile. (This is inherently safe, because it just makes is_constructible report accurate answers, and doesn't affect code that previously compiled.) WG21 has (gradually) taught pair/tuple to do this (constructors are now constrained on is_constructible for elements, and additionally is_convertible when they care about implicitness; previously they didn't ask is_constructible).

The relevant code in optional.hpp is:

template<class Expr> explicit optional ( Expr&& expr,

BOOST_DEDUCED_TYPENAME boost::disable_if_c<

(boost::is_base_of<optional_detail::optional_tag, BOOST_DEDUCED_TYPENAME boost::decay<Expr>::type>::value)

boost::is_same<BOOST_DEDUCED_TYPENAME boost::decay<Expr>::type, none_t>::value >::type* = 0

)

: base(boost::forward<Expr>(expr),boost::addressof(expr)) {optional_detail::prevent_binding_rvalue_ref_to_optional_lvalue_ref<T, Expr&&>();}

Unlike my reduced repro (where is_constructible<T, Expr> is the constraint corresponding to thing(forward<Expr>(expr))), I can't suggest an exact fix without knowing what optional_base's constructor is going to do with the provided arguments.

As an aside, the "prevent_binding" stuff should probably be expressed as a SFINAE constraint, not a static_assert.

Thanks, STL

[1] ​https://connect.microsoft.com/VisualStudio/feedback/details/2351203/vc14-2-ctp1-std-tuple-broken [2] ​http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html [3] ​http://cplusplus.github.io/LWG/lwg-active.html#2549

[akrzemi1]

Visual Studio for, I think, two releases supported dcltype bot no variadic templates. I really had this compiler in mind.

Whether this makes sense or not is one question, but the other question remains. It is not strictly in the scope of this library, but could you give me a macro that says if the is_constructible works correctly or falls back to a negative default? Currently I had to repeat the long #if from is_constructible.hpp, as I cannot rely on the 'safe' default.