Inadvertent conversions

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.

This entry was posted in programming and tagged , , . Bookmark the permalink.

10 Responses to Inadvertent conversions

  1. Mike says:

    rational(double) = delete;

    ?

    • gnzlbg says:

      What about:

      template <typename U> 
      struct rational 
      {
        template <typename T> 
          rational(T a, T b) = delete; // forbids conversions?
        rational(U a, U b) : ... { ... }
      };
      
      • @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.

  2. Marek says:

    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.

  3. szborows says:

    `= 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…

  4. Paweł says:

    Here’s my solution, based on fact that implicit convertion creates rvalue : http://ideone.com/pTsbiP

    • Thibaud says:

      Unfortunately, your rationale class is not copy constructible anymore. See ‘Too perfect forwarding’ on this blog.

  5. Paul says:

    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):

    Comment
    byu/mmmmario from discussion
    incpp

  6. Darrell Wright says:

    Wouldn’t adding explicit to the constructor Rational( T value ) resolve the issue?

    • The effect would be:

      rational<int> r1 = 1;   // (A) does not compile
      rational<int> r2 = 1.5; // (B) does not compile
      rational<int> r3(1);    // (C) works as expected
      rational<int> r4(1.5);  // (D) truncates 
      

      So, case (A) would no longer work (a regression) and case (D) would still work incorrectly.

Leave a comment

This site uses Akismet to reduce spam. Learn how your comment data is processed.