gh-93442: Make C++ version of _Py_CAST work with 0/NULL. by nascheme · Pull Request #93500

gh-93442: Make C++ version of _Py_CAST work with 0/NULL. by nascheme · Pull Request #93500 · python/cpython

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

Conversation

Add C++ overloads for _Py_CAST_impl() to handle 0/NULL. This will allow
C++ extensions that pass 0 or NULL to macros using _Py_CAST() to
continue to compile. Without this, you get an error like:

invalid ‘static_cast’ from type ‘int’ to type ‘_object*’

The modern way to use a NULL value in C++ is to use nullptr. However,
we want to not break extensions that do things the old way.

Co-authored-by: serge-sans-paille

Add C++ overloads for _Py_CAST_impl() to handle 0/NULL.  This will allow
C++ extensions that pass 0 or NULL to macros using _Py_CAST() to
continue to compile.  Without this, you get an error like:

    invalid ‘static_cast’ from type ‘int’ to type ‘_object*’

The modern way to use a NULL value in C++ is to use nullptr.  However,
we want to not break extensions that do things the old way.

Co-authored-by: serge-sans-paille

nascheme linked an issue

Jun 4, 2022

that may be closed by this pull request

corona10 changed the title ~~Make C++ version of _Py_CAST work with 0/NULL.~~ gh-93442: Make C++ version of _Py_CAST work with 0/NULL.

Jun 5, 2022

		// gh-93442: Pass 0 as NULL for PyObject*
		Py_XINCREF(0);
		Py_XDECREF(0);

Choose a reason for hiding this comment

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

nit


	Py_XINCREF(nullptr);
	Py_XDECREF(nullptr);

	Py_XINCREF(NULL);
	Py_XDECREF(NULL);

Thanks @nascheme for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this issue

Jun 5, 2022

…nGH-93500)

Add C++ overloads for _Py_CAST_impl() to handle 0/NULL.  This will allow
C++ extensions that pass 0 or NULL to macros using _Py_CAST() to
continue to compile.  Without this, you get an error like:

    invalid ‘static_cast’ from type ‘int’ to type ‘_object*’

The modern way to use a NULL value in C++ is to use nullptr.  However,
we want to not break extensions that do things the old way.

Co-authored-by: serge-sans-paille
(cherry picked from commit 8bcc3fa)

Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>

corona10 pushed a commit that referenced this issue

Jun 5, 2022

…h-93507)

Add C++ overloads for _Py_CAST_impl() to handle 0/NULL.  This will allow
C++ extensions that pass 0 or NULL to macros using _Py_CAST() to
continue to compile.  Without this, you get an error like:

    invalid ‘static_cast’ from type ‘int’ to type ‘_object*’

The modern way to use a NULL value in C++ is to use nullptr.  However,
we want to not break extensions that do things the old way.

Co-authored-by: serge-sans-paille
(cherry picked from commit 8bcc3fa)

Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>

Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>

Since there is a _Py_CAST_impl() implementation for nullptr, it would be nice to add a test on nullptr.

What is the purpose of _Py_CAST_impl(long int ptr)? Is it to pass a pointer as a number? Or is it to pass 0L or (long)0? Who uses such code?

I added a test for nullptr. The reason for the int overloads is because the C++ template type is created from the expression, not the type it is being coerced to. So, _Py_CAST(0) or _Py_CAST(NULL) expands the template with long int or similar as the type. I think the long int is due to what type C++ uses for a literal 0 value.

I'm a bit scared about how complicated _Py_CAST_impl() has become. AFAICT, it only exists to avoid a gcc warning. It's nice to avoid the warning but my concern is if there are more issues and the long term maintenance of that "magic" code. Since it seems to work, I guess we should keep it.

I'm a bit scared about how complicated _Py_CAST_impl() has become. AFAICT, it only exists to avoid a gcc warning.

It's more about generating "correct" code for a very tricky operation: convert any operation to PyObject*. With PEP 670, the limited C API version 3.11 simply removes the cast: you must pass PyOject* and only PyObject*. The complexity of the C++ implementation is more the cost of the past, keeping backward compatibility without emiting new C++ compiler warnings.

But you're right that my initial concern was fixing new C++ compiler warnings: to fix a regression. See #91320 (comment) for the rationale and why it's a regression.

I added a test for nullptr

Ah right, commit 713eb18: thanks!

test_cppext is new in Python 3.11! Previously, there was simply zero test on the C++ compatibility of the Python C API ;-)