Boost C++ Libraries: Ticket #12002: optional(Expr&&) is insufficiently constrained https://svn.boost.org/trac10/ticket/12002 <p> Hi, </p> <p> A user reported a bug (VSO<a class="missing ticket">#191303</a>/Connect<a class="missing ticket">#2351203</a>, see <a class="changeset" href="https://svn.boost.org/trac10/changeset/1" title="Import core sources for SVNmanger 0.38 ">[1]</a>) 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. </p> <p> Here's a reduced test case, which contains the relevant parts of boost::optional, and doesn't involve that bug in VC's tuple: </p> <p> C:\Temp&gt;type meow.cpp #include &lt;tuple&gt; #include &lt;type_traits&gt; #include &lt;utility&gt; using namespace std; </p> <p> struct none_t { }; </p> <p> template &lt;typename T&gt; struct optional { </p> <blockquote> <p> T thing; </p> </blockquote> <blockquote> <p> optional(none_t) </p> <blockquote> <p> : thing() { } </p> </blockquote> </blockquote> <blockquote> <p> template &lt;typename Expr, </p> <blockquote> <p> typename = typename enable_if&lt; </p> <blockquote> <p> !is_same&lt;typename decay&lt;Expr&gt;::type, none_t&gt;::value </p> </blockquote> </blockquote> </blockquote> <p> #ifdef FIX </p> <blockquote> <p> &amp;&amp; is_constructible&lt;T, Expr&gt;::value </p> </blockquote> <p> #endif </p> <blockquote class="citation"> <p> ::type&gt; </p> </blockquote> <blockquote> <p> explicit optional(Expr&amp;&amp; expr) </p> <blockquote> <p> : thing(forward&lt;Expr&gt;(expr)) { } </p> </blockquote> </blockquote> <p> }; </p> <p> int main() { </p> <blockquote> <p> tuple&lt;none_t&gt; one{}; </p> </blockquote> <blockquote> <p> tuple&lt;optional&lt;double&gt;&gt; two(one); </p> </blockquote> <p> } </p> <p> C:\Temp&gt;cl /EHsc /nologo /W4 meow.cpp meow.cpp meow.cpp(22): error C2440: 'initializing': cannot convert from 'std::tuple&lt;none_t&gt;' to 'double' [...context...] </p> <p> C:\Temp&gt;cl /EHsc /nologo /W4 /DFIX meow.cpp meow.cpp </p> <p> C:\Temp&gt; </p> <p> This is happening because I implemented N4387 (see <a class="changeset" href="https://svn.boost.org/trac10/changeset/2" title="Add Boost Disclaimer">[2]</a>) and the Proposed Resolution for LWG 2549 (see <a class="changeset" href="https://svn.boost.org/trac10/changeset/3" title="Tweak disclaimer text">[3]</a>). The PR is required to implement N4387 safely. </p> <p> 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&lt;A&gt; and the destination is tuple&lt;B&gt;, LWG 2549 determines whether A is constructible from tuple&lt;B&gt;. If it IS, then the converting copy ctor vanishes, so that tuple's perfect forwarding ctor (tuple(UTypes&amp;&amp;...)) can construct A from tuple&lt;B&gt;. Alternatively, if A ISN'T constructible from tuple&lt;B&gt;, 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&lt;B&gt;. </p> <p> The boost::optional bug is that optional(Expr&amp;&amp;) isn't sufficiently constrained, so it appears to be constructible (via is_constructible) from tuple&lt;none_t&gt;, even though the definition will explode. So LWG 2549 makes tuple's converting copy ctor vanish, and we end up attempting to construct optional&lt;double&gt; from tuple&lt;none_t&gt; which is doomed. </p> <p> 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). </p> <p> The fix that should be applied to boost::optional is to constrain optional(Expr&amp;&amp;) 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). </p> <p> The relevant code in optional.hpp is: </p> <blockquote> <p> template&lt;class Expr&gt; explicit optional ( Expr&amp;&amp; expr, </p> <blockquote> <p> BOOST_DEDUCED_TYPENAME boost::disable_if_c&lt; </p> <blockquote> <table class="wiki"> <tr>(boost::is_base_of&lt;optional_detail::optional_tag, BOOST_DEDUCED_TYPENAME boost::decay&lt;Expr&gt;::type&gt;::value) </tr></table> <p> boost::is_same&lt;BOOST_DEDUCED_TYPENAME boost::decay&lt;Expr&gt;::type, none_t&gt;::value &gt;::type* = 0 </p> </blockquote> </blockquote> <p> ) </p> <blockquote> <p> : base(boost::forward&lt;Expr&gt;(expr),boost::addressof(expr)) {optional_detail::prevent_binding_rvalue_ref_to_optional_lvalue_ref&lt;T, Expr&amp;&amp;&gt;();} </p> </blockquote> </blockquote> <p> Unlike my reduced repro (where is_constructible&lt;T, Expr&gt; is the constraint corresponding to thing(forward&lt;Expr&gt;(expr))), I can't suggest an exact fix without knowing what optional_base's constructor is going to do with the provided arguments. </p> <p> As an aside, the "prevent_binding" stuff should probably be expressed as a SFINAE constraint, not a static_assert. </p> <p> Thanks, STL </p> <p> <a class="changeset" href="https://svn.boost.org/trac10/changeset/1" title="Import core sources for SVNmanger 0.38 ">[1]</a> <a class="ext-link" href="https://connect.microsoft.com/VisualStudio/feedback/details/2351203/vc14-2-ctp1-std-tuple-broken"><span class="icon">​</span>https://connect.microsoft.com/VisualStudio/feedback/details/2351203/vc14-2-ctp1-std-tuple-broken</a> <a class="changeset" href="https://svn.boost.org/trac10/changeset/2" title="Add Boost Disclaimer">[2]</a> <a class="ext-link" href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html"><span class="icon">​</span>http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html</a> <a class="changeset" href="https://svn.boost.org/trac10/changeset/3" title="Tweak disclaimer text">[3]</a> <a class="ext-link" href="http://cplusplus.github.io/LWG/lwg-active.html#2549"><span class="icon">​</span>http://cplusplus.github.io/LWG/lwg-active.html#2549</a> </p> en-us Boost C++ Libraries /htdocs/site/boost.png https://svn.boost.org/trac10/ticket/12002 Trac 1.4.3 raad@… Fri, 19 Feb 2016 02:19:55 GMT cc set https://svn.boost.org/trac10/ticket/12002#comment:1 https://svn.boost.org/trac10/ticket/12002#comment:1 <ul> <li><strong>cc</strong> <span class="trac-author">raad@…</span> added </li> </ul> Ticket marci_r@… Fri, 19 Feb 2016 20:30:13 GMT cc changed https://svn.boost.org/trac10/ticket/12002#comment:2 https://svn.boost.org/trac10/ticket/12002#comment:2 <ul> <li><strong>cc</strong> <span class="trac-author">marci_r@…</span> added </li> </ul> Ticket akrzemi1 Sat, 20 Feb 2016 19:11:23 GMT <link>https://svn.boost.org/trac10/ticket/12002#comment:3 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/12002#comment:3</guid> <description> <p> Visual Studio for, I think, two releases supported <code>dcltype</code> bot no variadic templates. I really had this compiler in mind. </p> <p> 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 <code>#if</code> from is_constructible.hpp, as I cannot rely on the 'safe' default. </p> </description> <category>Ticket</category> </item> <item> <dc:creator>akrzemi1</dc:creator> <pubDate>Sat, 05 Mar 2016 22:33:17 GMT</pubDate> <title>status, milestone changed; resolution set https://svn.boost.org/trac10/ticket/12002#comment:4 https://svn.boost.org/trac10/ticket/12002#comment:4 <ul> <li><strong>status</strong> <span class="trac-field-old">new</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">fixed</span> </li> <li><strong>milestone</strong> <span class="trac-field-old">To Be Determined</span> → <span class="trac-field-new">Boost 1.61.0</span> </li> </ul> Ticket