Neil Girdhar added the comment:

Hi Steve:

I have limited expertise in most of these areas, but I looked at 
starunpack40.diff and have these comments:

* tests look to have good coverage of the feature (can't speak to coverage of 
the parser/compiler code)
* parsermodule.c changes comprehension handling, but I thought we pulled this 
out?

There was some code taken common in various places, but there should be no code 
for unpacking comprehensions left in.   Do you have a question about a specific 
line?

* why was dictobject.c.h added? I don't understand clinic thoroughly, but it 
seems a lot of new code for what was changed in dictobject.c

They weren't added.  They were moved by someone else.  The only changes are 
exposing a method.

* can the BUILD_(TUPLE|LIST)_UNPACK code in ceval.c share most of the 
processing? Or is there an unwanted perf impact to that?

Good idea.  I'll make that change.

* whoever applies the patch should regenerate importlib.h themselves - just a 
reminder

Otherwise, I didn't see anything that particularly scared me. Since we 
apparently don't have anyone willing and with the expertise to thoroughly check 
the patch, I'd vote for checking it in asap so it has more releases to bake 
before 3.5 final.

Thanks!

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue2292>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to