msg263297 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-04-13 02:37 |
It seems some code in _PyObject_GenericSetAttrWithDict is not necessary. There is no need to check data descriptor again using PyDescr_IsData. And the second if (f != NULL) {} will never function. |
|
|
msg263341 - (view) |
Author: Josh Rosenberg (josh.r) *  |
Date: 2016-04-13 16:57 |
LGTM. Had to check the definition of PyDescr_IsData to determine that checking the value from tp_descr_set for NULL was exactly what that macro does, but yeah, it looks like the first test was redundant, and f is never assigned outside that block, so the second block after the rest of the work is pointless; you'd never get there. |
|
|
msg263449 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-04-15 05:30 |
This code mirrors code in the Get function, but that function inspects tp_descr_get (not set) instead. As I understand it, this is the difference between “data” descriptors and “non-data” descriptors. So I think the change is okay. |
|
|
msg263463 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2016-04-15 09:44 |
LGTM. But there are other redundant code in _PyObject_GenericSetAttrWithDict(). Here is a patch that makes larger refactoring. |
|
|
msg263467 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2016-04-15 10:22 |
It's a better work. And the code looks simpler now. I test it with the test suite and none fails (though some tests are skipped due to platform). |
|
|
msg263478 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-04-15 11:38 |
Serhiy’s version looks good to me |
|
|
msg263479 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2016-04-15 11:44 |
I'll defer committing the patch until some progress in will be reached. May be _PyObject_GenericSetAttrWithDict() will have to rewrite again. |
|
|
msg263619 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-04-17 17:32 |
New changeset e5149789e4ea by Serhiy Storchaka in branch 'default': Issue #26745: Removed redundant code in _PyObject_GenericSetAttrWithDict. https://hg.python.org/cpython/rev/e5149789e4ea |
|
|
msg263620 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2016-04-17 17:33 |
Issue26767 looks too complex and resolving it needs much more rewriting. |
|
|