Skip to content

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jul 2, 2021

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. I have just few nitpicks to the tests.

@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Jul 3, 2021
@serhiy-storchaka serhiy-storchaka merged commit 1097384 into python:main Jul 3, 2021
@miss-islington
Copy link
Contributor

Thanks @Fidget-Spinner for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 3, 2021
(cherry picked from commit 1097384)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 3, 2021
@bedevere-bot
Copy link

GH-27002 is a backport of this pull request to the 3.10 branch.

@Fidget-Spinner Fidget-Spinner deleted the union_gc branch July 3, 2021 12:56
miss-islington added a commit that referenced this pull request Jul 3, 2021
(cherry picked from commit 1097384)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@pablogsal
Copy link
Member

result->args = dedup_and_flatten_args(args);
if (result->args == NULL) {
Py_DECREF(result);
PyObject_GC_Del(result);
Copy link
Member

@pablogsal pablogsal Jul 3, 2021

Choose a reason for hiding this comment

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

This is incorrect, this should still be Py_DECREF, PyObject_GC_Del cannot be called like that because it overrides a bunch of cleanups like the call to _Py_ForgetReference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants