Skip to content
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

Use Py_SET_SIZE() on Python 3.9.0a4 and newer #3639

Merged
merged 3 commits into from May 27, 2020
Merged

Use Py_SET_SIZE() on Python 3.9.0a4 and newer #3639

merged 3 commits into from May 27, 2020

Conversation

vstinner
Copy link
Contributor

Py_SIZE() must not be used as an l-value anymore in Python 3.9:
Py_SET_SIZE() must be used instead:

@vstinner
Copy link
Contributor Author

Maybe a __Pyx_Py_SET_SIZE() wrapper should be added which use use Py_SET_SIZE() if available, or use Py_SIZE() otherwise. But I don't know how to do that.

I don't know if tests/run/verbatiminclude.pyx test was valid or not. In case of doubt, I rename the test function. Does the test require that Py_SET_SIZE() already exists in Python.h? Or does it define a new function?

@scoder
Copy link
Contributor

scoder commented May 26, 2020

Thanks Victor!

Or does it define a new function?

This. And that's intentional in the test, it aims to define something in C. I think renaming the function there is the best way forward, as you did.

__Pyx_Py_SET_SIZE()

Yes, I think that would be the safer (and less intrusive) way to do this. There is a PythonCompatibility section in ModuleSetupCode.c. Just add it there, maybe below the __Pyx_PySequence_SIZE define.

@vstinner
Copy link
Contributor Author

Ok, I updated my PR to add a new _Pyx_SET_SIZE() function.

I also fixed Cython/Compiler/ModuleNode.py to avoid using Py_REFCNT() as an l-value.

I created this PR to make Cython compatible with my CPython change: python/cpython#20429

Cython/Compiler/ModuleNode.py Outdated Show resolved Hide resolved
Cython/Utility/ModuleSetupCode.c Outdated Show resolved Hide resolved
Add __Pyx_SET_SIZE() function: use Py_SET_SIZE() on Python 3.9.0a4
and newer, or use Py_SIZE() as an l-value on older Python versions.

Py_SIZE() must not be used as an l-value anymore in Python 3.9:
Py_SET_SIZE() must be used instead:

* https://bugs.python.org/issue39573
* https://docs.python.org/dev/c-api/structures.html#c.Py_SET_SIZE
Add __Pyx_SET_REFCNT() function: use Py_SET_REFCNT() on Python 3.9.0a4
and newer, or use Py_REFCNT() as an l-value on older Python versions.

Py_REFCNT() must not be used as an l-value anymore in Python 3.9:
Py_SET_REFCNT() must be used instead:

* https://bugs.python.org/issue39573
* https://docs.python.org/dev/c-api/structures.html#c.Py_SET_REFCNT

Use it in ModuleNode.generate_usr_dealloc_call():

* Replace ++Py_REFCNT(o) with __Pyx_SET_REFCNT(o, Py_REFCNT(o) + 1)
* Replace --Py_REFCNT(o) with __Pyx_SET_REFCNT(o, Py_REFCNT(o) - 1)
@vstinner
Copy link
Contributor Author

I squashed my commits to rewrite commit messages.

tests/run/verbatiminclude.pyx Outdated Show resolved Hide resolved
Cython/Utility/ModuleSetupCode.c Outdated Show resolved Hide resolved
Cython/Utility/Builtins.c Show resolved Hide resolved
Cython/Compiler/ModuleNode.py Show resolved Hide resolved
@vstinner
Copy link
Contributor Author

@scoder: I updated my PR to address your latest review.

@scoder
Copy link
Contributor

scoder commented May 26, 2020

Ok, let's see if travis gives her placet, then I'll merge this. Thanks Victor!

vstinner added a commit to vstinner/pythonci that referenced this pull request May 26, 2020
@vstinner
Copy link
Contributor Author

@scoder: Travis CI is happy. The two PyPy jobs failed, but I don't think that it's related to this PR.

@scoder scoder merged commit 63ab284 into cython:master May 27, 2020
@scoder scoder added this to the 0.29.19 milestone May 27, 2020
scoder pushed a commit that referenced this pull request May 27, 2020
…-3639)

* Add __Pyx_SET_SIZE() function: use Py_SET_SIZE() on Python 3.9.0a4
and newer, or use Py_SIZE() as an l-value on older Python versions.

Py_SIZE() must not be used as an l-value anymore in Python 3.9:
Py_SET_SIZE() must be used instead:

* https://bugs.python.org/issue39573
* https://docs.python.org/dev/c-api/structures.html#c.Py_SET_SIZE

* Add __Pyx_SET_REFCNT() function: use Py_SET_REFCNT() on Python 3.9.0a4
and newer, or use Py_REFCNT() as an l-value on older Python versions.

Py_REFCNT() must not be used as an l-value anymore in Python 3.9:
Py_SET_REFCNT() must be used instead:

* https://bugs.python.org/issue39573
* https://docs.python.org/dev/c-api/structures.html#c.Py_SET_REFCNT

Use it in ModuleNode.generate_usr_dealloc_call():

* Replace ++Py_REFCNT(o) with __Pyx_SET_REFCNT(o, Py_REFCNT(o) + 1)
* Replace --Py_REFCNT(o) with __Pyx_SET_REFCNT(o, Py_REFCNT(o) - 1)
@vstinner vstinner deleted the set_size branch May 27, 2020 12:46
@vstinner
Copy link
Contributor Author

Thanks for your great advices and also for merging quickly my change! Long life to the new opaque C API :-D

Oh, I tried to backport my fix 0.29.x branch, but you already did, thanks!

@vstinner
Copy link
Contributor Author

@scoder: I see that the fix is part of the Cython 0.29.19 milestone https://github.com/cython/cython/milestone/79 but Cython 0.29.19 was released 7 days ago?

Do you plan to release Cython 0.29.20 which will include this fix to make it compatible with future Python 3.10?

@scoder
Copy link
Contributor

scoder commented May 28, 2020 via email

@vstinner
Copy link
Contributor Author

vstinner commented Jun 3, 2020

Hum, 54bad41#diff-32655df5c2097cca93b52a2ff7583bef merge duplicated the macros. They are now defined twice:

#if PY_VERSION_HEX >= 0x030900A4

Extract of the code:

#if PY_VERSION_HEX >= 0x030900A4
  #define __Pyx_SET_REFCNT(obj, refcnt) Py_SET_REFCNT(obj, refcnt)
  #define __Pyx_SET_SIZE(obj, size) Py_SET_SIZE(obj, size)
#else
  #define __Pyx_SET_REFCNT(obj, refcnt) Py_REFCNT(obj) = (refcnt)
  #define __Pyx_SET_SIZE(obj, size) Py_SIZE(obj) = (size)
#endif


#if PY_VERSION_HEX >= 0x030900A4
  #define __Pyx_SET_REFCNT(obj, refcnt) Py_SET_REFCNT(obj, refcnt)
  #define __Pyx_SET_SIZE(obj, size) Py_SET_SIZE(obj, size)
#else
  #define __Pyx_SET_REFCNT(obj, refcnt) Py_REFCNT(obj) = (refcnt)
  #define __Pyx_SET_SIZE(obj, size) Py_SIZE(obj) = (size)
#endif

@scoder: Is it a mistake in a merge?

@scoder
Copy link
Contributor

scoder commented Jun 3, 2020

Interesting. Yeah, thanks for noticing @vstinner. I removed the duplication in 359f89a.

sgn added a commit to sgn/cypari2 that referenced this pull request Sep 22, 2022
From Python 3.9, Python provides Py_SET_SIZE to set size of Object.
From Python 3.11, Py_SIZE is no longer a macro, it's converted
to a function, thus its return value is now prvalue.

Cython provides a function to set size from 0.29.20 [1].

Let's use that instead.

[1]: cython/cython#3639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants