Skip to content
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.

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

gh-90868: Add _PyStaticObject_CheckRefcnt() function #99261

Merged
merged 1 commit into from
Nov 9, 2022
Merged

gh-90868: Add _PyStaticObject_CheckRefcnt() function #99261

merged 1 commit into from
Nov 9, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Nov 8, 2022

Add _PyStaticObject_CheckRefcnt() function to make _PyStaticObjects_CheckRefcnt() shorter. Use
_PyObject_ASSERT_FAILED_MSG() to log the object causing the fatal error.

@vstinner
Copy link
Member Author

vstinner commented Nov 8, 2022

@ericsnowcurrently @kumaraditya303: This PR makes _PyStaticObjects_CheckRefcnt() more readable and shorter, what do you think?

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

I have one suggestion. Otherwise this looks good.

Comment on lines 390 to 397
static inline void
_PyStaticObject_CheckRefcnt(PyObject *obj) {
if (Py_REFCNT(obj) < _PyObject_IMMORTAL_REFCNT) {
_PyObject_ASSERT_FAILED_MSG(obj,
"immortal object has less refcnt than expected "
"_PyObject_IMMORTAL_REFCNT");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Rather that being part of the generated code, you could put this directly in pycore_global_objects_fini_generated.h, before the /* The following is auto-generated by Tools/build/generate_global_objects.py. */ marker line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha, I didn't notice that only some parts of this file are generated. Ok, I added my new function at the top.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Add _PyStaticObject_CheckRefcnt() function to make
_PyStaticObjects_CheckRefcnt() shorter. Use
_PyObject_ASSERT_FAILED_MSG() to log the object causing the fatal
error.
@vstinner
Copy link
Member Author

vstinner commented Nov 8, 2022

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner vstinner merged commit 0124b5d into python:main Nov 9, 2022
@vstinner vstinner deleted the staticobj_check_refcnt branch November 9, 2022 07:40
@vstinner
Copy link
Member Author

vstinner commented Nov 9, 2022

Merged, thanks for the review.

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.

3 participants