Boost C++ Libraries: Ticket #10990: cpp_rational is not nothrow move constructible

John Maddock — Sat, 07 Feb 2015 17:57:32 GMT

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: https://github.com/boostorg/multiprecision/commit/33630bc407b044a98cd8c3b0dce93ca22a01a002

Sun, 08 Feb 2015 00:41:14 GMT

Replying to 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!

status changed; resolution set

John Maddock — Sun, 08 Feb 2015 18:22:17 GMT

status new → closed
resolution → fixed

Thanks, you're right of course, many more fixes in https://github.com/boostorg/multiprecision/commit/9927d49cb9d87c022c3ac49b3c8d84ce55ab7dc4.

Aside: I find it deeply frustrating that noexcept allows you to shoot yourself in the foot like this - the very least the compiler could do is issue a warning in the case that a noexcept function calls a non-noexcept one, otherwise I fear this is painfully easy to break!

Please reopen if you spot any other issues.