Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(85982)

#29300: Modify the _struct module to use FASTCALL and Argument Clinic

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by vstinner
Modified:
2 years, 6 months ago
Reviewers:
storchaka+cpython
CC:
haypo, larry, inada.naoki, skrah, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 7

Patch Set 3 #

Total comments: 17

Patch Set 4 #

Total comments: 8

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Patch Set 8 #

Total comments: 5

Patch Set 9 #

Patch Set 10 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Modules/clinic/_struct.c.h View 1 2 3 4 5 6 7 8 9 4 chunks +51 lines, -18 lines 0 comments Download
Modules/_struct.c View 1 2 3 4 5 6 7 8 9 12 chunks +52 lines, -65 lines 0 comments Download

Messages

Total messages: 11
storchaka
http://bugs.python.org/review/29300/diff/19839/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19839/Modules/_struct.c#newcode1530 Modules/_struct.c:1530: buffer: object Could the Py_buffer converter be used? http://bugs.python.org/review/29300/diff/19839/Modules/_struct.c#newcode1566 ...
2 years, 6 months ago #1
victor.stinner_gmail.com
http://bugs.python.org/review/29300/diff/19839/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19839/Modules/_struct.c#newcode1530 Modules/_struct.c:1530: buffer: object On 2017/01/26 19:03:03, storchaka wrote: > Could ...
2 years, 6 months ago #2
storchaka
http://bugs.python.org/review/29300/diff/19845/Doc/library/struct.rst File Doc/library/struct.rst (right): http://bugs.python.org/review/29300/diff/19845/Doc/library/struct.rst#newcode81 Doc/library/struct.rst:81: The function now accepts keyword arguments. I think this ...
2 years, 6 months ago #3
victor.stinner_gmail.com
http://bugs.python.org/review/29300/diff/19845/Doc/library/struct.rst File Doc/library/struct.rst (right): http://bugs.python.org/review/29300/diff/19845/Doc/library/struct.rst#newcode81 Doc/library/struct.rst:81: The function now accepts keyword arguments. On 2017/01/27 10:49:06, ...
2 years, 6 months ago #4
storchaka
http://bugs.python.org/review/29300/diff/19845/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19845/Modules/_struct.c#newcode1443 Modules/_struct.c:1443: On 2017/01/27 15:39:17, haypo wrote: > On 2017/01/27 10:49:06, ...
2 years, 6 months ago #5
victor.stinner_gmail.com
http://bugs.python.org/review/29300/diff/19845/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19845/Modules/_struct.c#newcode1443 Modules/_struct.c:1443: On 2017/01/27 16:05:02, storchaka wrote: > On 2017/01/27 15:39:17, ...
2 years, 6 months ago #6
victor.stinner_gmail.com
http://bugs.python.org/review/29300/diff/19846/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19846/Modules/_struct.c#newcode102 Modules/_struct.c:102: struct_format_converter(PyObject *format, PyObject **p_format) On 2017/01/27 16:05:02, storchaka wrote: ...
2 years, 6 months ago #7
storchaka
https://bugs.python.org/review/29300/diff/19845/Modules/_struct.c File Modules/_struct.c (right): https://bugs.python.org/review/29300/diff/19845/Modules/_struct.c#newcode1443 Modules/_struct.c:1443: On 2017/01/27 16:37:14, haypo wrote: > On 2017/01/27 16:05:02, ...
2 years, 6 months ago #8
victor.stinner_gmail.com
https://bugs.python.org/review/29300/diff/19846/Modules/_struct.c File Modules/_struct.c (right): https://bugs.python.org/review/29300/diff/19846/Modules/_struct.c#newcode2232 Modules/_struct.c:2232: buffer: Py_buffer > I hate bad keyword names for ...
2 years, 6 months ago #9
victor.stinner_gmail.com
http://bugs.python.org/review/29300/diff/19894/Modules/_struct.c File Modules/_struct.c (right): http://bugs.python.org/review/29300/diff/19894/Modules/_struct.c#newcode91 Modules/_struct.c:91: converter = 'cache_struct_converter' Even if the current code LGTM, ...
2 years, 6 months ago #10
storchaka
2 years, 6 months ago #11
http://bugs.python.org/review/29300/diff/19894/Modules/_struct.c
File Modules/_struct.c (right):

http://bugs.python.org/review/29300/diff/19894/Modules/_struct.c#newcode91
Modules/_struct.c:91: converter = 'cache_struct_converter'
On 2017/02/02 14:35:48, haypo wrote:
> Even if the current code LGTM, I would prefer to initialized the field to
NULL,
> just in case, since the cleanup code is always callede, even if the converter
> wasn't called.
> 
> I suggest to add:
> 
> c_default = "NULL"

Done.

http://bugs.python.org/review/29300/diff/19894/Modules/_struct.c#newcode2079
Modules/_struct.c:2079: PyDict_Clear(cache);
On 2017/02/02 14:35:48, haypo wrote:
> Do we have a LRU type usable at the C level? It would be more efficient than
> such simple caching algorithm.

No, we have not. Maybe in future simple LRU could be implemented using the
ordering of the dict type (issue28239).
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+