Re: [Boost-bugs] [Boost C++ Libraries] #10990: cpp_rational is not nothrow move constructible

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #10990: cpp_rational is not nothrow move constructible
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2015-02-08 00:41:14

#10990: cpp_rational is not nothrow move constructible
-------------------------------------+-------------------------------------
  Reporter:  Taras Morozovsky        |      Owner:  johnmaddock
  <taras.morozovsky@…>               |     Status:  new
      Type:  Bugs                    |  Component:  multiprecision
 Milestone:  To Be Determined        |   Severity:  Problem
   Version:  Boost 1.57.0            |   Keywords:  multiprecision
Resolution:                          |  cpp_rational move constructor
                                     |  noexcept
                                     |  is_nothrow_move_constructible
                                     |  rational_adaptor
-------------------------------------+-------------------------------------

Comment (by Taras Morozovsky <taras.morozovsky@…>):

 Replying to [comment:1 johnmaddock]:
 > Thanks for the catch, I need to do some more testing of the other types
 in the library but for now here's the patch for this this specific case

 Thank you for your reply!

 Please, keep in mind that arguments received by rvalue reference
 parameters are themselves lvalues. And so, in order to enable moving from
 them, they must be cast to rvalues once again. Without it, copy operations
 will be invoked instead, which may be dangerous since they may throw
 exceptions (std::bad_alloc in particular) when we promise that current
 operation will never throw.

 That's why ...''': m_value(o.m_value) {}''' at line 60 and ...''':
 m_value(o) {}''' at line 61 should be turned into ...''':
 m_value(static_cast<rational_type&&>(o.m_value)) {}''' and ...''':
 m_value(static_cast<IntBackend&&>(o)) {}''' respectively.

 Considering line 61, I believe this '''std::declval<integer_type>()'''
 must be '''std::declval<IntBackend>()''' instead, since a rational_type
 (m_value) is being constructed from an rvalue reference to IntBackend (o),
 not from a number<IntBackend> (which integer_type is).

 And one more thing I noticed: the condition inside BOOST_NOEXCEPT_IF at
 line 62. Now, it contains an expression '''std::declval<rational_type>() =
 std::declval<rational_type>()''' inside the noexcept operator. I think, it
 should be '''std::declval<rational_type&>() =
 std::declval<rational_type>()''' instead (with an lvalue reference on the
 left-hand side), because m_value is itself an lvalue at line 64 where it
 is being assigned to.

 Hope this will help someway!

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