Issue 26745: Redundant code in _PyObject_GenericSetAttrWithDict (original) (raw)

Created on 2016-04-13 02:37 by xiang.zhang, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
_PyObject_GenericSetAttrWithDict.patch xiang.zhang,2016-04-13 02:37 review
_PyObject_GenericSetAttrWithDict_2.patch serhiy.storchaka,2016-04-15 09:44 review
Messages (9)
msg263297 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) 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) * (Python triager) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2016-04-15 11:38
Serhiy’s version looks good to me
msg263479 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) (Python triager) 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) * (Python committer) Date: 2016-04-17 17:33
Issue26767 looks too complex and resolving it needs much more rewriting.
History
Date User Action Args
2022-04-11 14:58:29 admin set github: 70932
2016-04-17 17:33:08 serhiy.storchaka set status: open -> closedresolution: remind -> fixedmessages: +
2016-04-17 17:32:15 python-dev set nosy: + python-devmessages: +
2016-04-15 11:44:59 serhiy.storchaka set priority: normal -> lowmessages: + assignee: serhiy.storchakaresolution: remindstage: patch review -> commit review
2016-04-15 11:38:08 martin.panter set messages: +
2016-04-15 10:22:41 xiang.zhang set messages: +
2016-04-15 09:44:37 serhiy.storchaka set files: + _PyObject_GenericSetAttrWithDict_2.patchtype: enhancementmessages: +
2016-04-15 05:30:19 martin.panter set nosy: + martin.pantermessages: + stage: patch review
2016-04-13 16:57:35 josh.r set nosy: + josh.rmessages: +
2016-04-13 05:21:50 serhiy.storchaka set nosy: + serhiy.storchaka
2016-04-13 05🔞22 xiang.zhang set nosy: + pitrou, benjamin.peterson
2016-04-13 02:37:14 xiang.zhang create