[issue4136] merge json library with simplejson 2.0.3

[Martin v. Löwis, 2009-01-04 13:22:28]

http://codereview.appspot.com/7311/diff/1/8
File Lib/json/decoder.py (right):

http://codereview.appspot.com/7311/diff/1/8#newcode55
Line 55: def py_scanstring(s, end, encoding=None, strict=True, _b=BACKSLASH,
_m=STRINGCHUNK.match):
This function should get some comments what all the various cases are
(preferably speaking with the terms of JSON spec, i.e. chars, char, ...)

http://codereview.appspot.com/7311/diff/1/8#newcode71
Line 71: _append(content)
# 3 cases: end of string, control character, escape sequence

http://codereview.appspot.com/7311/diff/1/8#newcode76
Line 76: msg = "Invalid control character {0!r} at".format(esc)
esc isn't assigned until a few lines later. Is this really correct?

http://codereview.appspot.com/7311/diff/1/8#newcode104
Line 104: raise ValueError
No message?

http://codereview.appspot.com/7311/diff/1/8#newcode107
Line 107: raise ValueError
No message?

http://codereview.appspot.com/7311/diff/1/8#newcode111
Line 111: m = unichr(uni)
What's the purpose of m?

http://codereview.appspot.com/7311/diff/1/8#newcode127
Line 127: nextchar = s[end:end + 1]
Why not s[end]? Add comment if this is necessary.

http://codereview.appspot.com/7311/diff/1/8#newcode132
Line 132: nextchar = s[end:end + 1]
Likewise. There are more places where it does slicing, but also places where it
does indexing, in this function.

http://codereview.appspot.com/7311/diff/1/8#newcode290
Line 290: following strings: -Infinity, Infinity, NaN.
This sounds like an incompatible change.

http://codereview.appspot.com/7311/diff/1/8#newcode317
Line 317: def raw_decode(self, s, idx=0):
That looks like an incompatible change

http://codereview.appspot.com/7311/diff/1/9
File Modules/_json.c (right):

http://codereview.appspot.com/7311/diff/1/9#newcode196
Line 196: output_size *= 2;
You might want to check for integer overflow here.

http://codereview.appspot.com/7311/diff/1/9#newcode215
Line 215: ascii_escape_str(PyObject *pystr)
Please attach a comment to each function, telling what the function does.

http://codereview.appspot.com/7311/diff/1/9#newcode733
Line 733: "..."
Some text should probably be added here.

http://codereview.appspot.com/7311/diff/1/9#newcode1320
Line 1320: if ((idx + 3 < length) && str[idx + 1] == 'u' && str[idx + 2] == 'l'
&& str[idx + 3] == 'l') {
Is this really faster than a strncmp?

http://codereview.appspot.com/7311/diff/1/9#newcode1528
Line 1528: PyTypeObject PyScannerType = {
I think scanner objects should participate in cyclic gc.

http://codereview.appspot.com/7311/diff/1/9#newcode2025
Line 2025: "make_encoder",       /* tp_name */
That is a confusing type name. How about "Encoder"?

[bob.ippolito, 2009-01-05 01:28:18]

http://codereview.appspot.com/7311/diff/1/8
File Lib/json/decoder.py (right):

http://codereview.appspot.com/7311/diff/1/8#newcode55
Line 55: def py_scanstring(s, end, encoding=None, strict=True, _b=BACKSLASH,
_m=STRINGCHUNK.match):
Commented in the next patch.

http://codereview.appspot.com/7311/diff/1/8#newcode71
Line 71: _append(content)
Commented in the next patch

http://codereview.appspot.com/7311/diff/1/8#newcode76
Line 76: msg = "Invalid control character {0!r} at".format(esc)
This is a bug in the exception handling code, fixed in the next patch.

http://codereview.appspot.com/7311/diff/1/8#newcode104
Line 104: raise ValueError
Exception is caught at the except block and re-raised with a message. Next patch
unrolls this so it's not confusing.

http://codereview.appspot.com/7311/diff/1/8#newcode107
Line 107: raise ValueError
Exception is caught at the except block and re-raised with a message. Next patch
unrolls this so it's not confusing.

http://codereview.appspot.com/7311/diff/1/8#newcode111
Line 111: m = unichr(uni)
Renamed m to char in the next patch.

http://codereview.appspot.com/7311/diff/1/8#newcode127
Line 127: nextchar = s[end:end + 1]
commented in next patch (only once). s[end] can raise an IndexError with bad
input, s[end:end+1] returns an empty string on underflow, which is caught later
with a more helpful error message.

http://codereview.appspot.com/7311/diff/1/8#newcode132
Line 132: nextchar = s[end:end + 1]
commented in next patch (only once). s[end] can raise an IndexError with bad
input, s[end:end+1] returns an empty string on underflow, which is caught later
with a more helpful error message.

http://codereview.appspot.com/7311/diff/1/8#newcode290
Line 290: following strings: -Infinity, Infinity, NaN.
Not practically speaking. The documented purpose of this callback is "This can
be used to raise an exception if invalid JSON numbers are encountered.". I've
never seen it used to handle None, True, False in a different manner. That was
more of an implementation detail than anything else, and that is fixed by this
patch. Existing implementations of this callback will simply have dead code
since they will never be called with "null", "true" or "false" anymore.

http://codereview.appspot.com/7311/diff/1/8#newcode317
Line 317: def raw_decode(self, s, idx=0):
It is a compatible change.

http://codereview.appspot.com/7311/diff/1/9
File Modules/_json.c (right):

http://codereview.appspot.com/7311/diff/1/9#newcode196
Line 196: output_size *= 2;
_PyString_Resize checks for integer overflow, so it would explode there just
fine. The next patch changes this slightly to avoid unnecessary calls to
_PyString_Resize when the size didn't actually change, but doesn't do any
explicit integer overflow checking

http://codereview.appspot.com/7311/diff/1/9#newcode215
Line 215: ascii_escape_str(PyObject *pystr)
Done in the next patch

http://codereview.appspot.com/7311/diff/1/9#newcode733
Line 733: "..."
Done in the next patch.

http://codereview.appspot.com/7311/diff/1/9#newcode1320
Line 1320: if ((idx + 3 < length) && str[idx + 1] == 'u' && str[idx + 2] == 'l'
&& str[idx + 3] == 'l') {
Probably not, but strncmp doesn't work for PyUnicode and the same code is
repeated there.

http://codereview.appspot.com/7311/diff/1/9#newcode1528
Line 1528: PyTypeObject PyScannerType = {
I don't think it's possible to cause a cycle using the documented APIs, since
the encoder is created and thrown away behind the scenes and never passed to
user code. Someone else can write that patch if it's necessary.

http://codereview.appspot.com/7311/diff/1/9#newcode2025
Line 2025: "make_encoder",       /* tp_name */
It's not a type that's ever exposed to user code, make_encoder is somewhat less
confusing because that's the name it's exposed as. I'll change it anyway though,
it doesn't really matter since this is all private API.