-
Notifications
You must be signed in to change notification settings - Fork 784
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
Fix compilation on nightly #147
Conversation
// add bindings to the generated python module | ||
// N.B: names: "librust2py" must be the name of the `.so` or `.pyd` file | ||
/// This module is implemented in Rust. | ||
#[py::modinit(rust2py)] | ||
#[pymodinit(rust2py)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
py::modinit
doesn't work without the proc_macro_path_invoc
feature, which isn't expected to be stabilized soon (unlike proc macros and specialization).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see
you should have permission to merge |
Yes, thanks! |
@fafhrd91 Do you have any policy on when to make pull requests and when to directly push to master? |
I don't really have any policy, but master branch is protected :) |
rust-lang/rust#50120 has made two breaking changes that break pyo3's compilation:
proc_macro_path_invoc
. This means that e.g.#[py::modinit(rust2py)]
fails to compile.rust2py
and not(rust2py)
.This pull request fixes both. I've tried to avoid introducing breaking changes and instead replaced
py::modinit
with apymodinit
alias. I wouldn't mind removing the alias or renaming the proc macros.