Skip to content

Frame ffi #5154

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
May 23, 2025
Merged

Frame ffi #5154

merged 18 commits into from
May 23, 2025

Conversation

codeguru42
Copy link
Contributor

@codeguru42 codeguru42 commented May 19, 2025 •
edited
Loading

Add pyframe declarations and move around declarations to match CPython as of this PR (3.13, I think?).

@codeguru42 codeguru42 mentioned this pull request May 19, 2025
Closed
@codeguru42
Copy link
Contributor Author

There are compiler errors in pyo3-ffi-check that aren't immediately clear to me how to fix. I'll work through them as I can. Any tips for what to look at would be appreciated.

@codeguru42
Copy link
Contributor Author

Update:

This is the command that fails in the pipeline from what I can tell:

cargo run --manifest-path=pyo3-ffi-check/Cargo.toml

Running this locally, doesn't fail for me....and as I type this, I see the pipeline is running against py 3.14, so maybe I'm running against a different version of python.

@davidhewitt
Copy link
Member

The 3.14 failures are known and not caused by this PR, sorry for the confusion.

I'll retry the 3.13 job, looks like possible flakiness

davidhewitt
davidhewitt reviewed May 20, 2025
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks, just a couple small tidy ups I saw.

pyo3-ffi/src/cpython/frameobject.rs
Comment on lines -23 to -25
#[repr(C)]
#[cfg(not(any(PyPy, GraalPy, Py_3_11)))]
pub struct PyFrameObject {
Copy link
Member

Choose a reason for hiding this comment

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

We should add a 5154.removed.md to note that we no longer offer access to the frame object internal structure on versions before 3.11.

pyo3-ffi/src/pytypedefs.rs
Comment on lines +1 to +2
// TODO: Created this file as part of fixing pyframe.rs and cpython/pyframe.rs
// TODO: Finish defining or moving declarations now in Include/pytypedefs.h
Copy link
Member

Choose a reason for hiding this comment

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

👍

@codeguru42
Copy link
Contributor Author

@davidhewitt back to you

davidhewitt
davidhewitt approved these changes May 23, 2025
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks!

@davidhewitt davidhewitt added this pull request to the merge queue May 23, 2025
Merged via the queue into PyO3:main with commit 2c2f39f May 23, 2025
92 of 95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt approved these changes

Assignees
No one assigned
Labels
CI-build-full
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@codeguru42 @davidhewitt @dsal3389