Remove Incref/Decref of Specific Immortal Objects

[ericsnowcurrently]

Feature or enhancement

Proposal:

In various places we incref or decref an object that we already know is a specific immortal object, like None. For example, see gh-117393. These operations are unnecessary and the (small) cost can be avoided.

We could simply drop the increfs and decrefs, but there is value in communicating that we would have done the op if the object weren't known to be immortal. (Furthermore, there's the very, very remote chance that some known-immortal object might be made mortal again some day, for some currently unknown reason. It would be quite hard at the point to know that we should add back in the incref or decref we had removed.)

Here are the options I see:

1. leave the existing code alone
2. replace the existing code with comments
3. replace the existing code with a noop macro (in the internal API)

We probably wouldn't make all the changes we could find all at once, but could at the least point to this issue for the decision on what should be done. If we do add a macro then we'd do it relative to this issue.

Personally, I'd go with the macro. It would communicate the intent to readers just as well as the existing code does or as a comment would. I imagine it would look something like the following:

#ifdef Py_DEBUG
# define Py_INCREF_IMMORTAL(OBJ) \
    assert(_Py_IsImmortal(OBJ)
# define Py_DECREF_IMMORTAL(OBJ) \
    assert(_Py_IsImmortal(OBJ)
#else
# define Py_INCREF_IMMORTAL(OBJ)
# define Py_DECREF_IMMORTAL(OBJ)
#endif

For None we could be even more explicit:

#define Py_INCREF_NONE() \
    Py_INCREF_IMMORTAL(Py_None)
#define Py_DECREF_NONE() \
    Py_DECREF_IMMORTAL(Py_None)

// or

#define Py_INCREF_NONE(OBJ) \
    do { \
        assert(OBJ == Py_None) \
        Py_INCREF_IMMORTAL(OBJ) \
    } while (0)
#define Py_DECREF_NONE(OBJ) \
    do { \
        assert(OBJ == Py_None) \
        Py_DECREF_IMMORTAL(OBJ) \
    } while (0)

CC @eduardo-elizondo

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

[itamaro]

The sentiment makes sense to me, but the proposed naming makes me pause.
"inc/dec ref immortal", just from the name, looks like an oxymoron, and a hacky way to "force" manipulating the refcnt of an immortal object (e.g. "I know what I'm doing, ignore the fact this object is immortal and go ahead with modifying its refcnt").
Unfortunately I don't have a less confusing name to suggest for this...

[ericsnowcurrently]

Yeah, I'm open to other names. At the least, I wanted to capture the intention..

[eduardo-elizondo]

I'm more supportive of the macro - it also gives the flexibility to have extra assertions in debug more as you've outlined in your example