8000 BUG: do not decref borrowed refs for extobj and axes by mhvk · Pull Request #11244 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: do not decref borrowed refs for extobj and axes #11244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

BUG: do not decref borrowed refs for extobj and axes #11244

wants to merge 1 commit into from

Conversation

mhvk
Copy link
Contributor
@mhvk mhvk commented Jun 4, 2018 •
edited
Loading

The request by @eric-wieser for more tests in #11018 keep on yielding somewhat unrelated errors. Here, an overzealous XDECREF on borrowed values.

@mhvk mhvk added this to the 1.15.0 release milestone Jun 4, 2018
@mhvk mhvk force-pushed the ufunc-parsing-wrong-decref branch from 0088c3f to 4689d8b Compare June 4, 2018 18:42
@mhvk
Copy link
Contributor Author
mhvk commented Jun 4, 2018

Test passed on first round; next round only added some comments so should be all OK to review.

@mhvk mhvk requested a review from eric-wieser June 4, 2018 18:42
@eric-wieser
Copy link
Member

Can you add comments to the output args of get_ufunc_arguments, indicating which references are borrowed, and which are new?

@mhvk mhvk force-pushed the ufunc-parsing-wrong-decref branch from 4689d8b to fb62e9f Compare June 5, 2018 13:01
@mhvk
Copy link
Contributor Author
mhvk commented Jun 5, 2018

@eric-wieser - OK, done.

One question: shall I just give each their own reference? Right now it is a mess, and I'm not sure there is any real benefit to passing down those borrowed references.

@eric-wieser
Copy link
Member

That sounds like a pretty good idea to me

eric-wieser
eric-wieser reviewed Jun 6, 2018
View reviewed changes
numpy/core/src/umath/ufunc_object.c
* out_extobj: borrowed ref.
* out_typetup: new ref.
* out_wheremask: new ref.
* out_axes: borrowed ref.
Copy link
Member
@eric-wieser eric-wieser Jun 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

What about out_op? nevermind, I see that's above

I'd be inclined to put these on the same line as the parameter definitions, to reduce the chance of the comments going out of sync

@mhvk mhvk mentioned this pull request Jun 6, 2018
Merged
@mhvk
Copy link
Contributor Author
mhvk commented Jun 6, 2018

closing this in favour of #11257, where I just ensure that all have their own reference.

@mhvk mhvk closed this Jun 6, 2018
@mhvk mhvk deleted the ufunc-parsing-wrong-decref branch June 6, 2018 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser left review comments

Assignees
No one assigned
Labels
00 - Bug component: numpy._core
Projects
None yet
Milestone
1.15.0 release
Development

Successfully merging this pull request may close these issues.
2 participants
@mhvk @eric-wieser
0