Skip to content

Doc: int -> int or Py_ssize_t #18663

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

Merged
merged 1 commit into from
Feb 26, 2020
Merged

Doc: int -> int or Py_ssize_t #18663

merged 1 commit into from
Feb 26, 2020

Conversation

methane
Copy link
Member

@methane methane commented Feb 26, 2020

No description provided.

@miss-islington
Copy link
Contributor

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-18665 is a backport of this pull request to the 3.8 branch.

@bedevere-bot
Copy link

GH-18666 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <[email protected]>
miss-islington added a commit that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <[email protected]>
miss-islington added a commit that referenced this pull request Feb 26, 2020
(cherry picked from commit 57c7a0b)

Co-authored-by: Inada Naoki <[email protected]>
@vstinner
Copy link
Member

I'm not sure if this change is correct: the function uses int or Py_ssize_t depending on #define PY_SSIZE_T_CLEAN.

Are you suggesting to stop supporting C extension which don't use PY_SSIZE_T_CLEAN?

See the note:

For all # variants of formats (s#, y#, etc.), the type of the length argument (int or Py_ssize_t) is controlled by defining the macro PY_SSIZE_T_CLEAN before including Python.h. If the macro was defined, length is a Py_ssize_t rather than an int. This behavior will change in a future Python version to only support Py_ssize_t and drop int support. It is best to always define PY_SSIZE_T_CLEAN.

https://docs.python.org/dev/c-api/arg.html

@methane
Copy link
Member Author

methane commented Apr 24, 2020

I'm not sure if this change is correct: the function uses int or Py_ssize_t depending on #define PY_SSIZE_T_CLEAN.

As far as I know, it is correct. All # format uses int or Py_ssize_t depending on PY_SSIZE_T_CLEAN.

Are you suggesting to stop supporting C extension which don't use PY_SSIZE_T_CLEAN?

Yes. We raise DeprecationWarning when # is used without PY_SSIZE_T_CLEAN now.
I want to remove int support in the future. But I'm not sure we can remove it in 3.10.

(cc @serhiy-storchaka )

@serhiy-storchaka
Copy link
Member

It is definitely an improvement. Maybe we could add a footnote in all these sites to clarify that the type is Py_ssize_t when PY_SSIZE_T_CLEAN is defined and int otherwise, but there is a large note documenting this above the list of format units.

As for removing int support, I think we should not haste. The first version with runtime warnings is not achieved even the beta stage. We may to make warnings more quiet or use other mechanisms to control them if they will be too annoying. 3.10 and even 3.11 is too early to remove int support. I think we should start this when versions older then 3.9 become unsupported. I.e. in 3.12, 3.13 or later.

@methane
Copy link
Member Author

methane commented Apr 24, 2020

The first version with runtime warnings is not achieved even the beta stage.

Python 3.8 implements the runtime warning. Python 3.9 will be the second version of runtime warning.
https://github.com/python/cpython/pull/12473/files#diff-77c703d9a958f6a4b0dc2f692b3fd5b3

Static checker will be better, but I don't know how we can achieve it.

@serhiy-storchaka
Copy link
Member

Sorry, I thought it is a 3.9 feature. In any case 3.10 is too earlier for breaking C API change. Especially with reducing the time between releases.

Maybe PyCharm or VS Code will start to complain about PyArg_Parse() without PY_SSIZE_T_CLEAN and provide refactoring tools to insert PY_SSIZE_T_CLEAN and convert size types automatically.

@vstinner
Copy link
Member

Python 3.8 implements the runtime warning

In that case, IMO it's fine to start to raise an error in Python 3.10: we would have two releases (3.8 and 3.9) with the deprecation warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants