Boost C++ Libraries: Ticket #10639: lexical_cast(string) wrong in C++11

cc set

John Maddock — Fri, 10 Oct 2014 17:38:59 GMT

cc pbristow@… added

I had a quick look at this, and can confirm the issue - I'm really pretty surprised to see that the code is parsing the number itself rather than relying on std::num_get or whatever. Here's the thing: you absolutely cannot do this correctly (base 10 to 2 conversion) without arbitrary precision arithmetic - indeed in the worst case there is basically no limit to how many digits you may need in order to carry out the conversion correctly (though such cases are extremely rare, as in they will never ever occur in practice!). In fact getting this right is really bloody hard - Boost.Multiprecision has an algorithm under boost/multiprecision/cpp_bin_float/io.hpp based on MPFR's largely brute force approach, but honestly I wouldn't use that either for convert-to-built-in-type. There's more information at http://www.exploringbinary.com/real-c-rounding-is-perfect-gcc-now-converts-correctly/ which shows that even many respected std lib's often don't get this right in certain cases. I'm adding Paul Bristow to the CC list, as I know he has an interest in this.

status changed

Antony Polukhin — Sat, 11 Oct 2014 09:40:47 GMT

status new → assigned

The problem seems to be much worse than it looks like.

CLANG-3.4 and GCC-4.8.2 produce same results in C++03 and C++11 mode. Moreover, code in LexicalCast that does the conversion must work exactly the same way in C++11 and C++03. It is pretty simple and does not use real number types until the very end:

    const wide_result_t result = std::pow(static_cast<wide_result_t>(10.0), pow_of_10) * mantissa;
    value = static_cast<T>( has_minus ? (boost::math::changesign)(result) : result);

This makes me think that there's probably some precision error in std::pow.

Unfortunately I have no access to GCC-4.9 and Clang-3.5 at this moment, so I can not investigate this issue further. Please, could someone do it?

Thanks for the link! I'll put the test cases from it to the lexical cast auto tests and in case of errors will fallback to something like std::num_get. Maybe even with tests passing fallback to num_get will be done: current algo heavily relies on hardware precision and does not work in some cases (issue #6975).

John Maddock — Sat, 11 Oct 2014 10:37:08 GMT

Given:

const wide_result_t result = std::pow(static_cast<wide_result_t>(10.0), pow_of_10) * mantissa;

Then you have two floating point operations - which is to say, even if std::pow is accurate to 0.5ulp, and the multiplication likewise, you can still be wrong to 1ulp in the final result. Note that this is true even if long double is wider than double due to the "double rounding" problem: http://www.exploringbinary.com/double-rounding-errors-in-floating-point-conversions/.

You are correct that your code is the same in C++03 and C++11 modes which makes me wonder what's changed - my guess is that because of the issues outlined above, your code will be very susceptible to choice of floating-point registers used, and/or the level of compiler optimization used. Which is to say, the compiler only has to output slightly different code at the machine level, and stuff which worked before - largely by accident - will now break.

Fun isn't it? ;-)

HTH, John.

Antony Polukhin — Sat, 11 Oct 2014 12:10:15 GMT

Replying to johnmaddock:

Given:
const wide_result_t result = std::pow(static_cast<wide_result_t>(10.0), pow_of_10) * mantissa;
Then you have two floating point operations - which is to say, even if std::pow is accurate to 0.5ulp, and the multiplication likewise, you can still be wrong to 1ulp in the final result. Note that this is true even if long double is wider than double due to the "double rounding" problem: http://www.exploringbinary.com/double-rounding-errors-in-floating-point-conversions/.

Seems like a final nail into the coffin of my naive implementation. That's sad, std::num_get and others work slow because of memory allocations or do not respect locale specific separators. I'll force lexical cast to use std::stream based conversions, but this change possibly won't get its way into the 1.57 release (significant change that requires more testing).

You are correct that your code is the same in C++03 and C++11 modes which makes me wonder what's changed - my guess is that because of the issues outlined above, your code will be very susceptible to choice of floating-point registers used, and/or the level of compiler optimization used. Which is to say, the compiler only has to output slightly different code at the machine level, and stuff which worked before - largely by accident - will now break.

There's almost no chance that two compilers maintained by two different teams will change code generation between two minor releases at the same time only for the same specific set of input options.

This looks more like a regression in Standard Library implementation. As I understand in both test cases (Clang and GCC) the same Standard Library was used which makes it the first candidate for inspection.

Fun isn't it? ;-)

It makes me think that libc++ developers never stop laughing because of such fun... :-)

status, milestone changed; resolution set

Antony Polukhin — Mon, 09 Mar 2015 09:37:12 GMT

status assigned → closed
resolution → fixed
milestone To Be Determined → Boost 1.58.0