GeezerVison

I rebased my PR and I reimplemented Py_Is() as a macro.

And I added unit tests.

@markshannon: so are you against the principle of adding this macro and using it? Or can I merge it?

@corona10 @serhiy-storchaka @methane @pitrou: Do you think that these 4 functions are useful? See https://bugs.python.org/issue43753 for the rationale. It's mostly about helping to write C extensions compatible with PyPy, and prepare the bright future of HPy.

Can this be added to the stable ABI even though it's a macro?

Can this be added to the stable ABI even though it's a macro?

I don't see the point of adding it to the ABI in CPython right now. Can you please elaborate? I mostly care about the API.

Raymond is concerned by the risk of a negative impact on performance: https://bugs.python.org/issue43753#msg390379 As I only care about the API, I'm perfectly fine with a macro.

PyPy is free to implement it as a function if they want.

But then people targetting the stable ABI can't use this API? If the goal is to maximize usage, then it should be part of the ABI...

But then people targetting the stable ABI can't use this API? If the goal is to maximize usage, then it should be part of the ABI...

Ok, I get your point. That's why I declared Py_NewRef() and Py_XNewRef() as static inline functions and regular functions. I modified tests on these functions to test the macros (static inline functions) and the function implementations.

I prepared a pull request on my pythoncapi_compat project to add Py_Is(), Py_IsNone(), Py_IsTrue() and Py_IsFalse() to Python 3.9 and older and to PyPy: python/pythoncapi-compat#8 ;-) I will merge it if this PR is merged.

I am +1 on adding Py_Is(). It can be quite useful for some use-cases.

Ok.

But other variations are not convinced to me.
If I understand your intention correctly, we may create all the variations of PyIsXXX for singleton objects. When considering adding is easy but deprecating and removing is hard, I would like to add Py_Is() API only.

I don't propose to add a variation for all singletons. Only for these 3 which are the common (None, True, False).

I proposed Py_IsNone(), Py_IsTrue(), Py_IsFalse() to make the transition to Py_Is() more pleasant. IMO if we only add Py_Is(), it's harder to read code which repeat Py_ prefix in Py_Is and in Py_None.

I removed Py_IsNone(), Py_IsTrue(), Py_IsFalse().

I also modified Python/ceval.c in this PR, so you can see the change in practice.

So what do you prefer? Ony Py_Is() or with 4 functions?

Example with the current C API:

if (x == Py_None) {
    return 1;
}
if (x == Py_True) {
    return 2;
}
if (x == Py_False) {
    return 3;
}

Only with Py_Is():

if (Py_Is(x, Py_None)) {
    return 1;
}
if (Py_Is(x, Py_True)) {
    return 2;
}
if (Py_Is(x, Py_False)) {
    return 3;
}

With Py_IsNone(), Py_IsTrue() and Py_IsFalse():

if (Py_IsNone(x)) {
    return 1;
}
if (Py_IsTrue(x)) {
    return 2;
}
if (Py_IsFalse(x)) {
    return 3;
}

I think having the 4 functions is ok. x is None has been a recommended idiom for at least 20 years.

After I wrote my comment with examples showing the 3 APIs, I changed my mind. I really want Py_IsNone(), Py_IsTrue(), Py_IsFalse().

Having to replace x == Py_None with Py_Is(x, Py_None) is annoying. The new code is longer and harder is read.

whereas replacing x == Py_None with Py_IsNone(x) makes the code 1 character shorter and IMO it's as simple, or even simpler.

Even if the 3 functions are just helper, I would like to add them.

#define Py_IsNone(x) ((x) == Py_None)
#define Py_IsTrue(x) ((x) == Py_True)
#define Py_IsFalse(x) ((x) == Py_False)

"Py_IsTrue(x): Test if an object is True."

Hum, I was confused by this description. I rewrote it mentioning the singleton word:

=> "Py_IsTrue(x): Test if an object is the True singleton."

IMO the description now avoids any possible confusion with PyObject_IsTrue(x).

@pablogsal: Oh, thank you for fixing my English :-) I addressed your review.

About exposing Py_Is() and Py_IsNone() as a regular function: please see @pitrou's previous comment.

The docs CI failed with:

[2] ../Misc/NEWS.d/next/Library/2021-04-10-03-30-36.[bpo-43478](https://bugs.python.org/issue43478).iZcBTq.rst:1: default role used

Does anyone understand what's wrong with my NEWS entry?

Add the :c:func:`Py_Is(x, y) <Py_Is>` function to test if the *x* object is the
*y* object, the same as ``x is y`` in Python. Add also the :c:func:`Py_IsNone`,
:c:func:`Py_IsTrue`, :c:func:`Py_IsFalse` functions to test if an object is,
respectively, the ``None`` singleton, the ``True`` singleton or the ``False``
singleton.
Patch by Victor Stinner.

The docs CI failed with:

[2] ../Misc/NEWS.d/next/Library/2021-04-10-03-30-36.[bpo-43478](https://bugs.python.org/issue43478).iZcBTq.rst:1: default role used

Does anyone understand what's wrong with my NEWS entry?

Add the :c:func:`Py_Is(x, y) <Py_Is>` function to test if the *x* object is the
*y* object, the same as ``x is y`` in Python. Add also the :c:func:`Py_IsNone`,
:c:func:`Py_IsTrue`, :c:func:`Py_IsFalse` functions to test if an object is,
respectively, the ``None`` singleton, the ``True`` singleton or the ``False``
singleton.
Patch by Victor Stinner.

I think this warning was due to #25335

I think this warning was due to #25335

Oh ok. I merged my PR in this case.

Thanks @erlend-aasland @pitrou @corona10 and @pablogsal for your useful reviews!