Alexandre Vassalotti added the comment:

Stefan, I took a quick look at your patch. There is a couple things that stands 
out.

First, I think the implementation of BINGLOBAL and BINGLOBAL_BIG should be 
moved to another patch. Adding a binary version opcode for GLOBAL is a separate 
feature and it should be reviewed independently. Personally, I prefer the 
STACK_GLOBAL opcode I proposed as it much simpler to implement, but I am biased.

Next, the patch's formatting should be fixed to conform to PEP 7 and PEP 8. 
Make sure the formatting is consistent with the surrounding code. In 
particular, comments should be full sentences that explains why we need this 
code. Avoid adding comments that merely say what the code does, unless the code 
is complex.

In addition, please replace the uses of PyUnicode_InternFromString with the 
_Py_IDENTIFIER as needed. The latter allow the static strings to be garbage 
collected when the module is deleted, which is friendlier to embedded 
interpreters. It is also lead to cleaner code.

Finally, the class method check hack looks like a bug to me. There are multiple 
solutions here. For example, we could fix class methods to be cached so they 
always have the same ID once they are created. Or, we could remove the 'is' 
check completely if it is unnecessary.

----------

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

Reply via email to