Modify

#12002 closed Feature Requests (fixed)

optional(Expr&&) is insufficiently constrained

Reported by: akrzemi1 Owned by: akrzemi1
Milestone: Boost 1.61.0 Component: optional
Version: Boost 1.61.0 Severity: Problem
Keywords: Cc: raad@…, marci_r@…

Description

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

Attachments (0)

Change History (4)

comment:1 Changed 21 months ago by raad@…

Cc: raad@… added

comment:2 Changed 21 months ago by marci_r@…

Cc: marci_r@… added

comment:3 Changed 21 months ago by 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.

comment:4 Changed 21 months ago by akrzemi1

Milestone: To Be DeterminedBoost 1.61.0
Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain akrzemi1.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.