Skip to content

Incompatible change in internal string representation (encountered with Rust bindings) (?) #128972

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

Closed
clin1234 opened this issue Jan 18, 2025 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) pending The issue will be closed if no feedback is provided

Comments

@clin1234
Copy link

clin1234 commented Jan 18, 2025

Not sure if it's the right place to ask, but here it is.

I'm attempting to introduce 3.14 support in pyo3 (PyO3/pyo3#4811). I've encountered test failures involving UCS and refcounts targeting Windows and Linux. Invoking cargo test --no-default-features --lib --tests, one example of failure is here: https://github.com/clin1234/pyo3/actions/runs/12834317749/job/35791414368. The Python version used for testing 3.14 is 3.14.0a4.

However, running the same command with Python 3.14.0a3 running on Fedora Rawhide locally, I have encountered no test failures whatsoever.

And yes, I have pinged @davidhewitt on this matter.

Linked PRs

@sobolevn
Copy link
Member

Hi, thanks for the report! Can you please try to come up with a reproducer that does not include rust, but only C-API?

@davidhewitt
Copy link
Contributor

I don't think it's a CPython problem.

PyO3 has to replicate the C API at the ABI level, so it's not a surprise when bumping python versions that if structures change we need to make adjustments.

The PyUnicode data access macros are a known problem for PyO3 and to be honest I think the better option might be to just remove our mapping of them. But we can discuss that downstream.

@sobolevn sobolevn added the pending The issue will be closed if no feedback is provided label Jan 18, 2025
@ZeroIntensity
Copy link
Member

I suspect the "offending" change would be #128196, but yeah--we won't guarantee any stability with the internals, especially in the alpha phase.

Though, I am having some thoughts about whether or not the structure of PyASCIIObject is actually internal, because we do technically expose the structure in the non-limited API.

cc @encukou for the stability rules here.

@encukou
Copy link
Member

encukou commented Jan 20, 2025

By my reading of PEP-387, backwards-incompatible changes to PyASCIIObject needs deprecation period (or SC exception). However, that would be C API changes: rewriting that header in Rust is, unfortunately, not covered by any guarantees.
IMO, CPython should still try to keep more stable than promised. But this is a bit of a tight spot regarding backwards compatibility and free-threading support.


I see #128196 preserves C API compatibility but introduces an implementation-defined C feature (short bitfields) to do it. I fear that this will bite us in the future.

@corona10
Copy link
Member

By my reading of PEP-387, backwards-incompatible changes to PyASCIIObject needs deprecation period (or SC exception). However, that would be C API changes: rewriting that header in Rust is, unfortunately, not covered by any guarantees.
IMO, CPython should still try to keep more stable than promised. But this is a bit of a tight spot regarding backwards compatibility and free-threading support.

If we have to guarantee object layout stuff, one possible negotiation point would be just change for the free-threaded build because it is kind of an experimental build, so some of the breaking changes will be acceptable. (And we already break ABI compatibility between the default build and the free-threaded build)

cc @colesbury

@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 4, 2025
encukou added a commit to encukou/cpython that referenced this issue Apr 28, 2025
… layout.

Add `_Py_ALIGN_AS` as per C API WG vote: capi-workgroup/decisions#61
This patch only adds it to free-threaded builds; the `#ifdef Py_GIL_DISABLED`
can be removed in the future.

Use this to revert `PyASCIIObject` memory layout for non-free-threaded builds.
The long-term plan is to deprecate the entire struct; until that happens
it's better to keep it unchanged, as courtesy to people that rely on it despite
it not being stable ABI.
encukou added a commit that referenced this issue May 2, 2025
…t. (GH-133085)

Add `_Py_ALIGN_AS` as per C API WG vote: capi-workgroup/decisions#61
This patch only adds it to free-threaded builds; the `#ifdef Py_GIL_DISABLED`
can be removed in the future.

Use this to revert `PyASCIIObject` memory layout for non-free-threaded builds.
The long-term plan is to deprecate the entire struct; until that happens
it's better to keep it unchanged, as courtesy to people that rely on it despite
it not being stable ABI.
@encukou
Copy link
Member

encukou commented May 2, 2025

Reverted to give everyone time to stop relying on these details. But, note that:

  • again, struct layout is not guaranteed to stay fixed across CPython versions, unless it's part of the Stable ABI including all members (like Py_buffer for example).
  • I plan to deprecate these structs entirely, after adding some missing API.

Thanks for the report!

@encukou encukou closed this as completed May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) pending The issue will be closed if no feedback is provided
Projects
None yet
Development

No branches or pull requests

7 participants