Conversation
arthurpaulino
left a comment
There was a problem hiding this comment.
There are non-trivial trade-offs at stake on this PR.
The ergonomic Rust types that mimic the memory layout of Lean objects are gone. But at least we don't need to keep the synchronization by hand, which is more principled.
The layer that abstracts unsafe operations is gone. Maybe we can redefine it in terms of this new API later.
Also, docs/ffi.md needs to be completely rewritten. Or deleted, if we no longer bother explaining our FFI infrastructure since it's now a straightforward Lean -> Rust dependency.
arthurpaulino
left a comment
There was a problem hiding this comment.
This patch is superb! Truly amazing work 🎉
I've left my last batch of comments. Also, docs/ffi.md needs to be either rewritten or deleted.
arthurpaulino
left a comment
There was a problem hiding this comment.
This is beautiful!
Maybe docs/ffi.md (or portions of it) can become the README for the lean-ffi crate.
But I'm already approving the diff as-is.
698dcd9 to
595b859
Compare
Replaces the C FFI with automatically generated Rust bindings to
lean.husing https://github.com/rust-lang/rust-bindgen, which bypasses the C indirection and allows Rust to call the internal Lean FFI functions directly without having to manually emulate the C datatype layout in Rust.The bindings are contained in a new
lean-fficrate, which also defines Rust wrapper types around the low-level*const c_voidtype exposed by Bindgen. These help to clarify type intent and encapsulation on the Rust side. Updated documentation can be found atdocs/ffi.md.The bindgen approach was inspired by @lenianiva's reverse FFI example.
Note
The
rust-lintCI job was merged withrust-testdue to significant overlap and quick execution time, as we now need a Lean install in the Rust jobs to runbindgen. Therefore the branch protection rule needs to be updated before merge