Skip to content

gh-128509: Add sys._is_immortal for identifying immortal objects#128510

Merged
kumaraditya303 merged 20 commits intopython:mainfrom
ZeroIntensity:is-immortal-sys
Jan 31, 2025
Merged

gh-128509: Add sys._is_immortal for identifying immortal objects#128510
kumaraditya303 merged 20 commits intopython:mainfrom
ZeroIntensity:is-immortal-sys

Conversation

@ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Jan 5, 2025

@ZeroIntensity
Copy link
Member Author

@vstinner I think this is ready now that #129182 is merged. Could you take a look when you get a chance?

# CRASHES _testcapi.is_immortal(NULL)

def test_sys(self):
self.immortal_checking(sys._is_immortal)
Copy link
Member

Choose a reason for hiding this comment

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

sys functions must be checked in test_sys.

Copy link
Member Author

Choose a reason for hiding this comment

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

I intentionally moved the test from test_sys to test_immortal so we could re-use the test case, but I've reverted it now. Are you ok with the redundancy?

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with redundancy. test_capi can be skipped if _testcapi is missing. It's not the case for test_sys. Both tests are useful.

sys__is_immortal_impl(PyObject *module, PyObject *op)
/*[clinic end generated code: output=c2f5d6a80efb8d1a input=83733fc356c78475]*/
{
return _Py_IsImmortal(op);
Copy link
Member

Choose a reason for hiding this comment

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

Why not using PyUnstable_IsImmortal()?

Copy link
Member Author

Choose a reason for hiding this comment

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

The two are pretty similar. Would you prefer I use the unstable API?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to use the public unstable API, yes.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. IMO it makes sense to have a way in Python to check if an object is immortal. Otherwise, developers are confused by surprising sys.getrefcount() values.

}
}


Copy link
Member

Choose a reason for hiding this comment

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

unrelated change, I suggest to leave this empty 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.

Good catch!

@vstinner
Copy link
Member

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@kumaraditya303 kumaraditya303 enabled auto-merge (squash) January 31, 2025 15:04
@kumaraditya303 kumaraditya303 merged commit 9ba281d into python:main Jan 31, 2025
42 checks passed
@ZeroIntensity ZeroIntensity deleted the is-immortal-sys branch January 31, 2025 16:16
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Feb 7, 2025
…cts (python#128510)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants