Skip to content

Properly add link attributes to extern static pointer bindings - #44

Open
emmatyping wants to merge 1 commit into
3.x-rust-in-cpythonfrom
emmatyping/fix-extern-static-parsing
Open

Properly add link attributes to extern static pointer bindings#44
emmatyping wants to merge 1 commit into
3.x-rust-in-cpythonfrom
emmatyping/fix-extern-static-parsing

Conversation

@emmatyping

Copy link
Copy Markdown

The previous manual parsing for cpython-sys was not robust to different bindgen outputs. Rather than manual parsing, we now use syn and prettyplease to parse/unparse the bindgen output. These are not actually new dependencies as bindgen already depends on them.

The previous implementation of parsing was not properly handling some bindgen output (e.g. everything could end up on one line), so linker attributes were not added to items like PyExc_TypeError, causing link-time errors.

This should fix the Windows build errors seen in #35.

The previous manual parsing for cpython-sys was
not robust to different bindgen outputs. Rather
than manual parsing, we now use syn and
prettyplease to parse the bindgen output.
@emmatyping
emmatyping requested a review from Eclips4 August 10, 2026 05:10
let syn::Item::ForeignMod(foreign_mod) = item else {
continue;
};
let [syn::ForeignItem::Static(static_item)] = foreign_mod.items.as_slice() else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it expected that these extern blocks have a single item only?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, bindgen will emit one extern block per global, e.g. (taken from a generated c_api.rs):

unsafe extern "C" {
    pub static mut PyLong_Type: PyTypeObject;
}
unsafe extern "C" {
    pub static mut PyBool_Type: PyTypeObject;
}

I'll add a comment because this is somewhat load bearing on how bindgen generates extern data from C.

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