Remove dependency of pyo3-macros-backend on pyo3-build-config#5809
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
|
Nice, looks like this cut the runtimes of the |
pyo3-macros-backend on pyo3-build-configpyo3-macros-backend on pyo3-build-config
|
This is ready for review; I'm pretty pleased with this, it seems to improve a couple of painful issues with the build system with only a small bump in complexity. |
Tpt
left a comment
There was a problem hiding this comment.
Nice to see improved compile times 🥳
| if let Some(kw) = &attr.options.dict { | ||
| assertions.extend(quote_spanned! { | ||
| kw.span() => #[allow(dead_code)] | ||
| const _: () = { |
There was a problem hiding this comment.
pedantic-nit: could this shorter version be enough?
const _: () = {
assert!(#pyo3_path::impl_::pyclass::DICT_SUPPORTED, "{}", #pyo3_path::impl_::pyclass::DICT_UNSUPPORTED_ERROR);
};and maybe even wrap it in a function like:
#[track_caller]
const fn assert_dict_supported() {
assert!(cfg!(any(not(Py_LIMITED_API), Py_3_9)),
"`dict` requires Python >= 3.9 when using the `abi3` feature");
}and just have
const _: () = { assert_dict_supported() };There was a problem hiding this comment.
Thanks, I did try functions but I missed the #[track_caller] attribute and without that the error messages show the function source, which I didn't like as much.
With #[track_caller], this works perfectly. Pushed, will merge 👍
| [package] | ||
| name = "pyo3" | ||
| version = "0.28.1" | ||
| version = "0.28.2" |
There was a problem hiding this comment.
Ooops this is from local testing, will merge release notes merge first! 🙈
Looks like this cuts a few minutes off the
clippyCI jobs - see #5809 (comment)This will also fix #4579
This changes the macros to emit the same code regardless of Python version, leaving the implementation in
pyo3to decide what to do with the code.I suspect that this will also improve the situation for users who are experiencing rebuilds as per #1708 (by making rebuilds less expensive)