Today I want to warn you about a bug in Boost.Rational library. This illustrates a certain danger connected with allowing convenience conversions from other types.
The library is used to represent rational numbers. A rational number is a pair of integers, which are interpreted as the nominator and denominator:
boost::rational<int> r(1, 2);
This represents the value 1/2. Additionally, for convenience, you can pass it only one integer:
boost::rational<int> q = 3;
This represents the rational number 3/1 (the denominator is assumed to be 1). A convenience function, it uses the common intuition that integral numbers are a subset of rational numbers, that 3 has the same value as 3/1. This appears to be fine.
Today, when trying to figure out why my program produces wrong numbers, I found the following code:
boost::rational<int> s = 2.5;
I was surprised to find out that as the result, the value of thus initialized s
is 2 (2/1). Check it yourself. The documentation of Boost.Rational explains in detail that because it is impossible to convert a double
to rational<int>
without the loss of information, the authors have decided not to define it (see here). Unfortunately, the compiler was clever enough to invent the conversion itself. It first converts double
to int
, the fractional part is discarded, and then uses the rational
’s convenience converting constructor from int
.
I leave the discussion on where the source of the problem is, and in what ways it could be fixed to the readers. One conclusion I draw from this example is that the interface of a library is not the set of the declarations of its member functions, but the set of expressions that are made valid. As we can see above these two sets can be different.
rational(double) = delete;
?
What about:
@gnzlbg: I altered your post a bit in order for the code to display right. I hope I preserved your intention. I put some instructions on how to put code in the comments in the About page (see here). The tool is inconvenient, but at least it is free.
I thik, there shall be an extra overload for `double`, `float` and `long double` (and maybe others) marked as `= delete`, so the compiler will fail sooner and notify the user about unexpected conversion.
`= delete` will work fine, as Mike and Marek say.
Interesting fact – clang (3.5, trunk 196051) issues a warning when this conversion happens, while gcc (4.8.2) stays quiet. clang > gcc for another time…
Here’s my solution, based on fact that implicit convertion creates rvalue : http://ideone.com/pTsbiP
Unfortunately, your rationale class is not copy constructible anymore. See ‘Too perfect forwarding’ on this blog.
Well implicit conversions can be avoided by using a template parameter and then you can constrain the template to avoid non-integral types. There’s an example here on my comment on reddit(since I can’t type code reliably in the comment section):
http://www.reddit.com/r/cpp/comments/2bqg9q/andrzej_inadvertent_conversions/cj8um4b
Wouldn’t adding explicit to the constructor Rational( T value ) resolve the issue?
The effect would be:
So, case (A) would no longer work (a regression) and case (D) would still work incorrectly.