Skip to content

bpo-32999: Revert GH-6002 (fc7df0e6)#6189

Merged
methane merged 2 commits into
python:masterfrom
methane:bpo32999-direct-mro
Mar 22, 2018
Merged

bpo-32999: Revert GH-6002 (fc7df0e6)#6189
methane merged 2 commits into
python:masterfrom
methane:bpo32999-direct-mro

Conversation

@methane

@methane methane commented Mar 22, 2018

Copy link
Copy Markdown
Member

bpo-33018 (GH-5944) fixed bpo-32999 too. So fc7df0e is not required
anymore. Revert it except test case.

https://bugs.python.org/issue32999

bpo-33018 (pythonGH-5944) fixed bpo-32999 too.  So fc7df0e is not required
anymore.  Revert it except test case.

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just one comment.

Comment thread Modules/_abc.c Outdated
end:
Py_DECREF(impl);
Py_XDECREF(mro);
Py_XDECREF(impl);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can stay DECREF, since impl can't be NULL here.

@methane methane merged commit f757b72 into python:master Mar 22, 2018
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@methane methane deleted the bpo32999-direct-mro branch March 22, 2018 12:52
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 22, 2018
bpo-33018 (pythonGH-5944) fixed bpo-32999 too.  So fc7df0e is not required
anymore.  Revert it except test case.
(cherry picked from commit f757b72)

Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
ilevkivskyi pushed a commit that referenced this pull request Mar 22, 2018
bpo-33018 (GH-5944) fixed bpo-32999 too.  So fc7df0e is not required
anymore.  Revert it except test case.
(cherry picked from commit f757b72)

Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
Comment thread Modules/_abc.c
assert(PyTuple_Check(mro));
for (pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) {
PyObject *mro_item = PyTuple_GET_ITEM(mro, pos);
if (mro_item == NULL) {

@zhangyangyu zhangyangyu Mar 22, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@methane Late review, this check is redundant or not right. To keep it, you need to set an exception in the block, otherwise you are returning NULL without an exception, then SystemError.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. It should be assert(mro_item != NULL). I'll fix it tomorrow.

jo2y pushed a commit to jo2y/cpython that referenced this pull request Mar 23, 2018
bpo-33018 (pythonGH-5944) fixed bpo-32999 too.  So fc7df0e is not required
anymore.  Revert it except test case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants