gh-128509: Add `PyUnstable_IsImmortal` for finding immortal objects by ZeroIntensity · Pull Request #129182 · python/cpython · GitHub

I've reused the test from my other PR.
The plan is to go for this, and then possibly go for gh-128509: Add sys._is_immortal for identifying immortal objects #128510 afterwards.

Issue: Add an API for determining if an object is immortal #128509

📚 Documentation preview 📚: https://cpython-previews--129182.org.readthedocs.build/en/129182/c-api/object.html#c.PyUnstable_IsImmortal

Co-authored-by: Victor Stinner <vstinner@python.org>

LGTM.

cc @colesbury @serhiy-storchaka

I think @encukou's review would also be helpful.

Suggested change

      
                return PyBool_FromLong(PyUnstable_IsImmortal(op));
          
                NULLABLE(op)
          
                return PyLong_FromLong(PyUnstable_IsImmortal(op));

This allows to test with NULL, and check if the result is not 0 or 1.

I think it makes sense to crash with NULL.

Does _testcapi.is_immortal(NULL) crash? If yes, add a comment, otherwise add a test.

If yes, add a comment

And ideally an assert to the implementation (as “executable documentation”).

If yes, add a comment

And ideally an assert to the implementation (as “executable documentation”).

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Co-authored-by: Petr Viktorin <encukou@gmail.com>

This reverts commit 6014587.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Did you test this?

is_immortal(None) checks if None is immortal, it doesn't crash.

I suggest to remove this comment. I don't think that it's worth it to bother with testing NULL, PyUnstable_IsImmortal() now starts with assert(op != NULL);. And it's interesting to test if None is immortal or not.

I suggest to apply my suggestions to be able to test with NULL.

That would break the test with None. I think as long as we have the assertion there, it's not worth explicitly testing it.

class TestCAPI(unittest.TestCase):
    def test_immortal_builtins(self):
        _testcapi.test_immortal_builtins()
    def test_immortal_small_ints(self):
_testcapi.test_immortal_small_ints()

There is a problem with existing tests: all methods called test_xxx are run automatically by test_capi.test_misc:

vstinner@mona$ ./python -m test test_capi -v -m test_immortal_small_ints
(...)
test_immortal_small_ints (test.test_capi.test_immortal.TestCAPI.test_immortal_small_ints) ... ok
test_immortal_small_ints (test.test_capi.test_misc.Test_testcapi.test_immortal_small_ints) ... ok
(...)

Tests are run twice. Can you fix these tests as part of your PR? Just use a different function name prefix, such as check_xxx.

Oh, I didn't know that Modules/_testcapi/immortal.c exists. Maybe put the new function there?

LGTM

ZeroIntensity added 5 commits January 22, 2025 09:20

Add PyUnstable_IsImmortal

58ac265

Add a test.

8a64537

Fix test.

4b55134

Add a whatsnew entry.

6bddf33

Add blurb

412a42b

ZeroIntensity requested a review from vstinner January 22, 2025 14:30

bedevere-app Bot added the awaiting review label Jan 22, 2025

bedevere-app Bot mentioned this pull request Jan 22, 2025

Add an API for determining if an object is immortal #128509

Closed

vstinner reviewed Jan 22, 2025

View reviewed changes

Comment thread Doc/c-api/object.rst Outdated

Comment thread Doc/c-api/object.rst Outdated

Comment thread Doc/c-api/object.rst Outdated

Comment thread Doc/c-api/object.rst Outdated

Comment thread Doc/c-api/object.rst Outdated

Comment thread Modules/_testcapimodule.c Outdated

ZeroIntensity and others added 4 commits January 22, 2025 12:06

Update object.rst

21cf2c4

Co-authored-by: Victor Stinner <vstinner@python.org>

Update object.rst

8d20c00

Co-authored-by: Victor Stinner <vstinner@python.org>

Remove some information.

831626c

Move test function.

422edf9

ZeroIntensity requested a review from vstinner January 22, 2025 21:16

vstinner approved these changes Jan 22, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Jan 22, 2025

serhiy-storchaka reviewed Jan 23, 2025

View reviewed changes

encukou reviewed Jan 23, 2025

View reviewed changes

ZeroIntensity and others added 4 commits January 23, 2025 08:03

Update object.c

6014587

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Update object.rst

48c974c

Co-authored-by: Petr Viktorin <encukou@gmail.com>

Add non-null assertion

dd19a02

Revert "Update object.c"

99e897f

This reverts commit 6014587.

serhiy-storchaka reviewed Jan 23, 2025

View reviewed changes

Comment thread Lib/test/test_capi/test_immortal.py Outdated

Update Lib/test/test_capi/test_immortal.py

0448daf

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka reviewed Jan 24, 2025

View reviewed changes

vstinner reviewed Jan 24, 2025

View reviewed changes

Prevent tests from running twice and move is_immortal()

5b949b1

Fix missing comma.

d1c9b1f

ZeroIntensity requested review from encukou and serhiy-storchaka January 24, 2025 13:29

vstinner approved these changes Jan 24, 2025

View reviewed changes

encukou merged commit 3fb5f6e into python:main Jan 27, 2025

bedevere-app Bot removed the awaiting merge label Jan 27, 2025

ZeroIntensity mentioned this pull request Jan 27, 2025

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

Merged

ZeroIntensity deleted the unstable-is-immortal branch January 27, 2025 15:42

+              static PyObject *
+              is_immortal(PyObject *self, PyObject *op)
+              {
+                  return PyBool_FromLong(PyUnstable_IsImmortal(op));

	return PyBool_FromLong(PyUnstable_IsImmortal(op));
	NULLABLE(op)
	return PyLong_FromLong(PyUnstable_IsImmortal(op));

+                      for non_immortal in non_immortals:
+                          with self.subTest(non_immortal=non_immortal):
+                              self.assertFalse(_testcapi.is_immortal(non_immortal))

gh-128509: Add `PyUnstable_IsImmortal` for finding immortal objects by ZeroIntensity · Pull Request #129182 · python/cpython · GitHub

Repository navigation

gh-128509: Add PyUnstable_IsImmortal for finding immortal objects - #129182

gh-128509: Add PyUnstable_IsImmortal for finding immortal objects#129182encukou merged 16 commits intopython:mainpython/cpython:mainfrom ZeroIntensity:unstable-is-immortalZeroIntensity/cpython:unstable-is-immortalCopy head branch name to clipboard

Conversation

ZeroIntensity commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

ZeroIntensity commented Jan 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-128509: Add PyUnstable_IsImmortal for finding immortal objects#129182
encukou merged 16 commits into
python:mainfrom
ZeroIntensity:unstable-is-immortal

ZeroIntensity commented Jan 22, 2025 •
edited
Loading