Noissue - implement a couple of common lexical_cast cases.

Add `lexical_cast converting a String value into an integer, Boolean or OwnedString.
So far the implementation is the bare minimum required to have it, in particular to let one implement https://codereview.adblockplus.org/29548581/ and https://codereview.adblockplus.org/29606600/, where it was extracted from.

Review: https://codereview.adblockplus.org/29714586

[hub, 2018-03-05 19:24:49]

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h
File compiled/String.h (right):

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h#n...
compiled/String.h:496: for (; pos < len && value[pos] >= u'0' && value[pos] <=
u'9'; ++pos)
If the condition `value[pos] >= u'0' && value[pos] <= u'9'` is false, then we
should have an error.

I'm comparing this to the behaviour of `boost::lexical_cast<>` where
`lexical_cast<int>(" 123")` is an error, so is `lexical_cast<int>("123 ")`,
because of the space.

This would also be consistent with `LexicalCastImpl<bool>::Convert()` above that
does a strict equality with `u"true"_str`.

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h#n...
compiled/String.h:498: // isDengerous is the optimization because there is no
need for some checks
isDangerous (typo)

[sergei, 2018-03-06 10:36:27]

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h
File compiled/String.h (right):

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h#n...
compiled/String.h:496: for (; pos < len && value[pos] >= u'0' && value[pos] <=
u'9'; ++pos)
On 2018/03/05 19:24:49, hub wrote:
> If the condition `value[pos] >= u'0' && value[pos] <= u'9'` is false, then we
> should have an error.
> 
> I'm comparing this to the behaviour of `boost::lexical_cast<>` where
> `lexical_cast<int>(" 123")` is an error, so is `lexical_cast<int>("123 ")`,
> because of the space.
> 
> This would also be consistent with `LexicalCastImpl<bool>::Convert()` above
that
> does a strict equality with `u"true"_str`.

Done, but I'm not sure about "123 ", anyway we can always fix it later.

https://codereview.adblockplus.org/29714586/diff/29714593/compiled/String.h#n...
compiled/String.h:498: // isDengerous is the optimization because there is no
need for some checks
On 2018/03/05 19:24:49, hub wrote:
> isDangerous (typo)

Done.

[hub, 2018-03-06 14:45:53]

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h
File compiled/String.h (right):

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h#n...
compiled/String.h:506: bool isDangerous = pos >=
std::numeric_limits<T>::digits10;
this check on the number of digits could also be done before the loop: if we
have more digits (`len` or `len - 1` if it has a sign) than the maximum it is an
error.

[sergei, 2018-03-06 15:41:32]

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h
File compiled/String.h (right):

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h#n...
compiled/String.h:506: bool isDangerous = pos >=
std::numeric_limits<T>::digits10;
On 2018/03/06 14:45:53, hub wrote:
> this check on the number of digits could also be done before the loop: if we
> have more digits (`len` or `len - 1` if it has a sign) than the maximum it is
an
> error.

This condition is satisfied but from a different perspective and the idea of
having isDangerous here is rather for optimization, namely in order to skip
overflow checks when there is no necessity in them.

std::numeric_limits<T>::digits10 is the maximum number of digits which can be
stored in an integer type T without loss. E.g. for int8_t the range is [-128,
127], so digits10 is equal to 2, it can safely store [-99, 99], similar for
other types. So while the length of a text represented number is less than
`digits10 + 1` it's safe to multiply it by 10 and add `digit`, however when the
length reaches digits10 one should check that there is no overflow. E.g. `value
= "300"`, `result == 30`, `pos = 2`, now we need to check that there is no
overflow before multiplying it by 10, in that example there is an overflow.
Another example is when the `value = 128`, it's safe to multiply 12 by 10 but
one should not add 8, one may add only a number from [0, 7]. Just in case, it's
covered in tests, but only for (u)int32_t.

If the number exceeds the valid range, e.g. "1111" for int8_t it will fail on
the check before multiplying by 10, `127 / 10 < 111` is true. It's also covered
in the tests.

It seems not a common case when a number is too big for parsing, so IMO we
should not check it before the for-loop additionally.

BTW the implementation could also take into account '-' but I tried to not
introduce additional conditions, so there is no such check because I thought
that negative numbers are not too common in our data either. BTW, because of
similar reasons there is a support of zeros at the beginning, there is just no
additional check for them and as the result there is such feature.

[hub, 2018-03-06 15:56:45]

LGTM

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h
File compiled/String.h (right):

https://codereview.adblockplus.org/29714586/diff/29715625/compiled/String.h#n...
compiled/String.h:506: bool isDangerous = pos >=
std::numeric_limits<T>::digits10;
On 2018/03/06 15:41:32, sergei wrote:
> On 2018/03/06 14:45:53, hub wrote:
> > this check on the number of digits could also be done before the loop: if we
> > have more digits (`len` or `len - 1` if it has a sign) than the maximum it
is
> an
> > error.
> 
> This condition is satisfied but from a different perspective and the idea of
> having isDangerous here is rather for optimization, namely in order to skip
> overflow checks when there is no necessity in them.
> 
> std::numeric_limits<T>::digits10 is the maximum number of digits which can be
> stored in an integer type T without loss. E.g. for int8_t the range is [-128,
> 127], so digits10 is equal to 2, it can safely store [-99, 99], similar for
> other types. So while the length of a text represented number is less than
> `digits10 + 1` it's safe to multiply it by 10 and add `digit`, however when
the
> length reaches digits10 one should check that there is no overflow. E.g.
`value
> = "300"`, `result == 30`, `pos = 2`, now we need to check that there is no
> overflow before multiplying it by 10, in that example there is an overflow.
> Another example is when the `value = 128`, it's safe to multiply 12 by 10 but
> one should not add 8, one may add only a number from [0, 7]. Just in case,
it's
> covered in tests, but only for (u)int32_t.
> 
> If the number exceeds the valid range, e.g. "1111" for int8_t it will fail on
> the check before multiplying by 10, `127 / 10 < 111` is true. It's also
covered
> in the tests.
> 
> It seems not a common case when a number is too big for parsing, so IMO we
> should not check it before the for-loop additionally.
> 
> BTW the implementation could also take into account '-' but I tried to not
> introduce additional conditions, so there is no such check because I thought
> that negative numbers are not too common in our data either. BTW, because of
> similar reasons there is a support of zeros at the beginning, there is just no
> additional check for them and as the result there is such feature.
> 

Acknowledged.