GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-89653: PEP 670: Fix PyUnicode_READ() cast by vstinner · Pull Request #92872 · python/cpython

Conversation

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member

_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner added skip news needs backport to 3.11 only security fixes labels

May 17, 2022

bedevere-bot added the awaiting core review label

May 17, 2022

vstinner mentioned this pull request

May 17, 2022

PyUnicode_READ() fails to build on C++: _Py_CAST() used with a constant type (const void*) #92800

Closed

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member Author

See bug report: #92800

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member Author

cc @erlend-aasland @da-woods

erlend-aasland reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

da-woods commented May 17, 2022

Copy link

Copy Markdown

Contributor

I'll have to wait until later to test and confirm that it's fixed the Cython bug.

Is it worth copying the test-case from #92818? (The actual fix here is probably more sensible though)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member Author

Is it worth copying the test-case from #92818? (The actual fix here is probably more sensible though)

Oh right, I added tests.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          gh-89653: PEP 670: Fix PyUnicode_READ() cast

75e5da7

_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member Author

I rebased the PR and I added more tests about the PyUnicode_READ() cast.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 reviewed

May 17, 2022

View reviewed changes

Comment thread

Lib/test/_testcppext.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 reviewed

May 17, 2022

View reviewed changes

Comment thread

Lib/test/_testcppext.cpp Outdated

		{
		PyObject *str = PyUnicode_FromString("abc");
		if (str == NULL) {
		return NULL;

kumaraditya303 May 17, 2022

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	return NULL;
	return nullptr;

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

kumaraditya303 reviewed

May 17, 2022

View reviewed changes

Comment thread

Lib/test/_testcppext.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

tacaswell commented May 17, 2022

Copy link

Copy Markdown

Contributor

I can confirm it fixes the cython bug.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tacaswell commented May 17, 2022

Copy link

Copy Markdown

Contributor

However on this branch greenlets is still broken:

Details

$ gcc -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/tcaswell/.virtualenvs/bleeding/include -I/home/tcaswell/.pybuild/bleeding/include/python3.12 -c src/greenlet/gre
enlet.cpp -o build/temp.linux-x86_64-3.12/src/greenlet/greenlet.o
In file included from /home/tcaswell/.pybuild/bleeding/include/python3.12/Python.h:38,
                 from src/greenlet/greenlet.cpp:14:
src/greenlet/greenlet_refs.hpp: In member function ‘void greenlet::refs::PyErrPieces::normalize()’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:781:41: note: in expansion of macro ‘_PyObject_CAST’
  781 | #  define PyType_Check(op) PyType_Check(_PyObject_CAST(op))
      |                                         ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyerrors.h:57:6: note: in expansion of macro ‘PyType_Check’
   57 |     (PyType_Check((x)) &&                                               \
      |      ^~~~~~~~~~~~
src/greenlet/greenlet_refs.hpp:993:17: note: in expansion of macro ‘PyExceptionClass_Check’
  993 |             if (PyExceptionClass_Check(type)) {
      |                 ^~~~~~~~~~~~~~~~~~~~~~
In file included from /home/tcaswell/.pybuild/bleeding/include/python3.12/Python.h:44:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::OwnedErrPiece’ to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:775:59: note: in definition of macro ‘PyType_FastSubclass’
  775 | #define PyType_FastSubclass(type, flag) PyType_HasFeature(type, flag)
      |                                                           ^~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’
  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
      |                               ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyerrors.h:61:25: note: in expansion of macro ‘Py_TYPE’
   61 |     PyType_FastSubclass(Py_TYPE(x), Py_TPFLAGS_BASE_EXC_SUBCLASS)
      |                         ^~~~~~~
src/greenlet/greenlet_refs.hpp:1003:22: note: in expansion of macro ‘PyExceptionInstance_Check’
 1003 |             else if (PyExceptionInstance_Check(type)) {
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp: In destructor ‘greenlet::ThreadState::~ThreadState()’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:266:59: note: in expansion of macro ‘_PyObject_CAST’
  266 | #  define PyObject_TypeCheck(ob, type) PyObject_TypeCheck(_PyObject_CAST(ob), type)
      |                                                           ^~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/methodobject.h:17:31: note: in expansion of macro ‘PyObject_TypeCheck’
   17 | #define PyCFunction_Check(op) PyObject_TypeCheck(op, &PyCFunction_Type)
      |                               ^~~~~~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:400:33: note: in expansion of macro ‘PyCFunction_Check’
  400 |                              && PyCFunction_Check(refs.at(0))
      |                                 ^~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:127:35: note: in expansion of macro ‘_PyObject_CAST’
  127 | #  define Py_REFCNT(ob) Py_REFCNT(_PyObject_CAST(ob))
      |                                   ^~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:401:33: note: in expansion of macro ‘Py_REFCNT’
  401 |                              && Py_REFCNT(refs.at(0)) == 2) {
      |                                 ^~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedObject’ {aka ‘greenlet::refs::BorrowedReference<_object, greenlet::refs::NoOpChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:581:29: note: in expansion of macro ‘_PyObject_CAST’
  581 |         PyObject *_py_tmp = _PyObject_CAST(op); \
      |                             ^~~~~~~~~~~~~~
src/greenlet/greenlet_thread_state.hpp:422:33: note: in expansion of macro ‘Py_CLEAR’
  422 |                                 Py_CLEAR(function_w);
      |                                 ^~~~~~~~
src/greenlet/greenlet.cpp: In function ‘PyObject* green_repr(greenlet::refs::BorrowedGreenlet)’:
/home/tcaswell/.pybuild/bleeding/include/python3.12/pyport.h:30:25: error: invalid cast from type ‘greenlet::refs::BorrowedGreenlet’ {aka ‘greenlet::refs::_BorrowedGreenlet<_greenlet, greenlet::refs::GreenletChecker>’} to type ‘const PyObject*’ {aka ‘const _object*’}
   30 |        const_cast<type>(reinterpret_cast<const type>(expr))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:107:28: note: in expansion of macro ‘_Py_CAST’
  107 | #define _PyObject_CAST(op) _Py_CAST(PyObject*, (op))
      |                            ^~~~~~~~
/home/tcaswell/.pybuild/bleeding/include/python3.12/object.h:136:31: note: in expansion of macro ‘_PyObject_CAST’
  136 | #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
      |                               ^~~~~~~~~~~~~~
src/greenlet/greenlet.cpp:2395:33: note: in expansion of macro ‘Py_TYPE’
 2395 |     const char* const tp_name = Py_TYPE(self)->tp_name;
      |                                 ^~~~~~~

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Replace NULL with nullptr

f462017

vstinner commented May 17, 2022

Copy link

Copy Markdown

Member Author

However on this branch greenlets is still broken:

If you consider that it's a regression, please open a separated issue. It seems to be a different warning/error.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner merged commit e6fd799 into python:main

May 17, 2022

miss-islington commented May 17, 2022

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the awaiting core review label

May 17, 2022

vstinner deleted the unicode_read_cast branch

May 17, 2022 17:20

bedevere-bot commented May 17, 2022

Copy link

Copy Markdown

GH-92884 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.11 only security fixes label

May 17, 2022

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

May 17, 2022


          pythongh-89653: PEP 670: Fix PyUnicode_READ() cast (pythonGH-92872)

60e987f

_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.
(cherry picked from commit e6fd799)

Co-authored-by: Victor Stinner <[email protected]>

miss-islington added a commit that referenced this pull request

May 17, 2022


          gh-89653: PEP 670: Fix PyUnicode_READ() cast (GH-92872)

1df4550

_Py_CAST() cannot be used with a constant type: use _Py_STATIC_CAST()
instead.
(cherry picked from commit e6fd799)

Co-authored-by: Victor Stinner <[email protected]>

Reviewers

kumaraditya303

kumaraditya303 left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.