Use addr_of_mut in pyo3-ffi #2172
Conversation
pyo3-ffi/build.rs
Outdated
| Ok(()) | ||
| } | ||
|
|
||
| fn rustc_minor_version() -> Option<u32> { |
There was a problem hiding this comment.
PyO3 itself already uses the rustversion, wouldn't it make sense to apply it here as well?
There was a problem hiding this comment.
The rustversion crate? That is only a dev dependency.
There was a problem hiding this comment.
Indeed, wouldn't also just be build-dependency if used in the build script here?
I understand that it is meant to be used directly for conditional compilation, but that could also be applied to the println! invocation in the build script to avoid manually parsing the version.
Or at least could we somehow deduplicate the parsing code between the build scripts of pyo3 and pyo3-ffi?
There was a problem hiding this comment.
Last time I discussed it @davidhewitt (I think?) didn't want to add dependencies, so I didn't do it here. I would prefer not to reinvent the wheel but I also don't want to add more macro crates as dependencies.
There was a problem hiding this comment.
I feel kind of split between deduplicating the code - I don't like duplication either but it's not a whole lot and it doesn't do anything clever, so i don't think the cost of duplication is great here. And we'll just delete it all next time we bump msrv.
There was a problem hiding this comment.
Yes, I keep going back and forth whether we should add rustversion and refactor our build scripts. On the one hand, it might be nice to use the dependency to keep our code a bit simpler. On the other, it keeps our dependency tree leaner to just do it ourselves. I slightly prefer the current implementation, though only slightly so if you folks wanted to change to rustversion that's fine by me.
There was a problem hiding this comment.
To avoid the duplication could always shove it into pyo3-build-config as a #[doc(hidden)] API :P
There was a problem hiding this comment.
The current implementation is pretty simple. I'll just keep that.
To avoid the duplication could always shove it into pyo3-build-config as a #[doc(hidden)] API :P
Yeah, I'll do just that.
All these comparisons and getters were going through &mut references to mutable statics...