Boost C++ Libraries: Ticket #10109: Wrong overflow checking with uint_parser

Joel de Guzman — Tue, 10 Jun 2014 15:25:14 GMT

You are requesting for a unit parser but you give it an int? uint_parser, for performance reasons, does not test for wrap around -- which is what happens here. it is assumed that your type T does not wrap around as int does. perhaps we should be strict and test it with a static assert instead, but there's no generic way to know if T does or does not wrap around. The alternative, that you seem to be suggesting is to check for wrap, but that would penalise the majority of uses.

No this is not a bug. And I don't think it should be "fixed". The best we can do is document this precondition that T should not wrap around.

Tue, 10 Jun 2014 16:59:16 GMT

According to the docs, uint_parser is supposed to parse an unsigned value, it doesn't make any restriction on the type it will read the value into. In fact, you can also create a uint_parser<double>, and in that case int_extractor::call(Char, size_t, T&) will use a different implementation, that just adds the digits without checking for overflow.

As I wrote, the current implementations already checks for wrap around, by using std::numeric_limits<T>::max in

template<typename T, typename Char>
positive_accumulator::add(T&, Char, mpl::true_)

The only issue is that it's using the wrong T. It uses numeric_limits<unsigned int>, because it is passed an unsigned int&. If it used numeric_limits<signed int> with the very same algorithm it would probably work perfectly. It looks like it only involves template work, which means no performance impact for those who aren't using it.

Joel de Guzman — Tue, 10 Jun 2014 22:38:31 GMT

OK, if you have a patch, then I'll consider it. Please post a pull request to https://github.com/boostorg/spirit/tree/develop, making sure that all the tests pass.

(Yes, the docs does not give any restrictions on T at the moment, but if there's no optimal way to make it work with wrapping ints, then I'm inclined to make it an unchecked precondition. T == double is irrelevant because double does not wrap. You can also use saturating ints that do not wrap, for example).

Mon, 01 Jan 2018 18:20:43 GMT

Fixed in https://github.com/boostorg/spirit/pull/297

status changed; resolution set

Joel de Guzman — Mon, 19 Feb 2018 23:15:27 GMT

status new → closed
resolution → fixed