Skip to content

more cleanup of private FFI definitions#6042

Open
davidhewitt wants to merge 7 commits into
PyO3:mainfrom
davidhewitt:private-cleanup-3
Open

more cleanup of private FFI definitions#6042
davidhewitt wants to merge 7 commits into
PyO3:mainfrom
davidhewitt:private-cleanup-3

Conversation

@davidhewitt
Copy link
Copy Markdown
Member

Another batch of FFI cleanups following #5965

I added --document-private-items to the ffi-check rustdoc processing; this didn't catch functions (which I hoped it would) but it noticed that the PyTracebackObject definition did not expose the fields.

I also adjusted the implementation of PyErr_BadInternalCall to be more like the CPython headers, where it is a macro which accepts __FILE__ and __LINE__ information.

Comment thread pyo3-ffi/src/lib.rs
Comment on lines +391 to +394
$pub struct $name {
_data: (),
_marker: core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>,
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change was motivated by looking at the nomicon https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs - which documents this _data / _marker combination as the current way to do opaque types.

@davidhewitt davidhewitt mentioned this pull request May 13, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 13, 2026

Merging this PR will not alter performance

✅ 125 untouched benchmarks


Comparing davidhewitt:private-cleanup-3 (0c719bc) with main (2187c7e)

Open in CodSpeed

@davidhewitt davidhewitt requested a review from ngoldbaum May 24, 2026 06:26
Copy link
Copy Markdown
Contributor

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

Everything looks correct to me. Thank you for the ping to look this over, I missed it in the churn.

@ngoldbaum
Copy link
Copy Markdown
Contributor

That said there's a merge conflict now.

@davidhewitt davidhewitt enabled auto-merge May 26, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants