[Boost-bugs] [Boost C++ Libraries] #12002: optional(Expr&&) is insufficiently constrained

Subject: [Boost-bugs] [Boost C++ Libraries] #12002: optional(Expr&&) is insufficiently constrained
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2016-02-18 23:58:40


#12002: optional(Expr&&) is insufficiently constrained
------------------------------+----------------------
 Reporter: akrzemi1 | Owner: akrzemi1
     Type: Feature Requests | Status: new
Milestone: To Be Determined | Component: optional
  Version: Boost 1.61.0 | Severity: Problem
 Keywords: |
------------------------------+----------------------
 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

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/12002>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.

This archive was generated by hypermail 2.1.7 : 2017-02-16 18:50:19 UTC