Introduce jsontext.RawToken API for better number parsing #158
Conversation
This is consistent with int overflow and json v1
These can be reused in jsontext. Note that jsonwire.ParseFloat now returns inf for overflow. But this should not impact user-visible behavior, because we don't set value when overflow.
Decoder now returns RawToken instead of Token. RawToken exposes a new set of methods: ParseFloat(bits int) (float64, error) ParseInt(bits int) (int64, error) ParseUint(bits int) (uint64, error) They reuses the same logic of arshalers, intended to offers Decoder user a more efficient and convenient way to parse numbers. Compared with the original Token.Float, these methods properly return errors for overflow, etc. Compared with strconv, these methods may take advantage of the fact that json number has simpler syntax than Go. Compared with json.UnmarshalDecode, these methods avoids the cost of arshaler lookup and reflection. New jsontext.Raw and Token.Raw func are added to convert between RawToken and Token. The original Float, Int, Uint methods now only support the case where the Token is created by the correspond constructor. e.g., Int(1).Uint() will panic. This should ensure we will never get inaccurate values.
Similarly, Parsing "Infinity" as +Inf is also removed This feature is not standard and should not be enabled by default. If this is really desired, user can still write a wrapper that invokes Float or String conditionally. Instead of embedding the maybe undesired branches in jsontext. It is harder to get consistent API design with the new RawToken refactor. Now Token.Float() should only process tokens returned by jsontext.Float, we need to differentate infinity float and infinity string. So what kind should the infinity float be? - if '0': Good for that it cannot be used as object key, but can make Token.String() confusing; also confusing for that a '0' token actually encodes to string. - if '"': Need many extra branching in Token.Kind and Token.appendString. And the kind returned by jsontext.Float() is not consistent. So, just remove these.
|
Hi, thanks for the contribution. I don't see a compelling reason to increase the API surface by an extra type, several methods, and break symmetry between tok, ... = dec.ReadToken()
f, err := strconv.ParseFloat(tok.String(), 64)or val, ... = dec.ReadValue()
f, err := strconv.ParseFloat(string(val), 64)The In the future, if we want to add a checked form of parsing functionality, we can add |
If I draw it out, it is still pretty symmetric
Reasonable, I'm actually using this in my project. But as I say in PR description: "Compared with strconv, these methods may take advantage of the fact that json number has simpler syntax than Go"
Totally agreed. So these methods are preserved in Token type. I just want to prevent the usages like: tok, ... = dec.ReadToken()
f := tok.Float()This is almost certainly a misuse of API because it ignores error. I want to make this a compilation error. While
I add a new type so that we can make this a compilation error: Plus, the size of RawToken is only 16B, which is only half of Token. This should benefits decoder only workload. |
Decoder now returns RawToken instead of Token. RawToken exposes a new set of methods:
They reuses the same logic of arshalers, intended to offers Decoder user a more efficient and convenient way to parse numbers.
New jsontext.Raw and Token.Raw func are added to convert between RawToken and Token.
The original Float, Int, Uint methods now only support the case where the Token is created by the correspond constructor. e.g., Int(1).Uint() will panic. This should ensure we will never get inaccurate values.
Other behavior changes:
nonfiniteis still supported in json package.This is inspired by the idea in golang/go#63397 (reply in thread) . I expand the idea of @neild , and add a new RawToken type. Now
RawTokenis used in Decoding, it only haveParseFloatintended for decodeTokenis still used in Encoding,Token.Floatnow become a simple accessor to retrieve the value set byjsontext.Float.With this design, we give up support for any inaccurate cases. Then we can answer the edge cases that @dsnet raised in the discussion naturally:
strconv.ParseIntstrconv.ParseInt