classification
Title: Move PyInterpreterState into Include/internal/pycore_pystate.h
Type: Stage: patch review
Components: Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: eric.snow, ncoghlan, vstinner
Priority: normal Keywords: patch, patch, patch

Created on 2019-02-02 01:07 by eric.snow, last changed 2019-02-15 23:39 by eric.snow.

Pull Requests
URL Status Linked Edit
PR 11731 open eric.snow, 2019-02-02 01:24
Messages (5)
msg334733 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-02-02 01:07
In November Victor created the Include/cpython directory and moved a decent amount of public (but not limited) API there.  This included the PyInterpreterState struct.  I'd like to move it into the "internal" headers since it is somewhat coupled to the internal runtime implementation.  The alternative is extra complexity.  I ran into this while working on another issue.

Note that the docs indicate that all of the struct's fields are private (and I am not aware of any macros leaking fields into the stable ABI).  So moving it should not break anything (yeah, right!), and certainly not the stable ABI (Victor's move would have done that already).
msg334734 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-02-02 01:32
FWIW, I was hoping to the same for PyThreadState but it looks like 6 of its fields are exposed in "stable" header files via the following macros:

  # Include/object.h
  Py_TRASHCAN_SAFE_BEGIN
  Py_TRASHCAN_SAFE_END
  Include.ceval.h
  Py_EnterRecursiveCall
  Py_LeaveRecursiveCall
  _Py_MakeRecCheck
  Py_ALLOW_RECURSION
  Py_END_ALLOW_RECURSION

I'm not sure how that factors into the stable ABI (PyThreadState wasn't ever guarded by Py_LIMITED_API).  However, I didn't need to deal with it right now so I'm not going to go there. :)
msg335120 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-02-09 01:14
@Victor, do you see any problems with doing this?  It will help simplify other changes I'm working on.
msg335464 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-02-13 17:05
> @Victor, do you see any problems with doing this?  It will help simplify other changes I'm working on.

I'm quite sure that they are users of the PyInterpreterState structure outside CPython internals and stdlib, but I expect that the number is quite low.

Since internal headers are now installed (I modified "make install" for that) (but need to define Py_BUILD_CORE), it might be acceptable to force users of this structure to opt-in for internal headers.

Just make sure that we properly communicate on such backward incompatible changes:

* "C API Changes" section of https://docs.python.org/dev/whatsnew/3.8.html#porting-to-python-3-8
* mail to python-dev

The bpo-35810 also proposes a subtle backward incompatible change which makes me unhappy, but Stefan Behnel seems less scared than me, so maybe it will be fine.

Maybe we need to organize a collective effort to better communicate on our backward incompatible C API changes. The capi-sig mailing list may be a good channel for that. I asked to test some popular C extensions to check that they are not broken. If it's the case, we should help them to be prepared for the new C API.
msg335657 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-02-15 23:39
Thanks, Victor!

python-dev: https://mail.python.org/pipermail/python-dev/2019-February/156344.html

Also, my PR already has a What's New entry in the porting section. :)
History
Date User Action Args
2019-02-15 23:39:45eric.snowsetkeywords: patch, patch, patch

messages: + msg335657
2019-02-13 17:05:33vstinnersetkeywords: patch, patch, patch

messages: + msg335464
2019-02-09 01:45:42eric.snowsetkeywords: patch, patch, patch
nosy: + ncoghlan
2019-02-09 01:14:43eric.snowsetkeywords: patch, patch, patch

messages: + msg335120
2019-02-08 16:55:51eric.snowsetpull_requests: - pull_request11624
2019-02-08 16:55:34eric.snowsetpull_requests: - pull_request11625
2019-02-02 01:32:46eric.snowsetkeywords: patch, patch, patch

messages: + msg334734
2019-02-02 01:25:03eric.snowsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request11625
2019-02-02 01:24:58eric.snowsetkeywords: + patch
stage: needs patch -> needs patch
pull_requests: + pull_request11624
2019-02-02 01:24:52eric.snowsetkeywords: + patch
stage: needs patch -> needs patch
pull_requests: + pull_request11623
2019-02-02 01:07:08eric.snowcreate