Reduce the amount of code we generate from macros#6187
Open
alex wants to merge 6 commits into
Open
Conversation
davidhewitt
reviewed
Jul 5, 2026
davidhewitt
left a comment
Member
There was a problem hiding this comment.
Thanks, generally always keen to improve codegen and I'm not surprised there were low-hanging fruit here. A few thoughts scattered below.
Member
|
You probably need some type annotations to fix the never type fallback errors. |
Merging this PR will degrade performance by 24.75%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
Slims the code generated by #[pyclass], #[pymethods], and #[pyfunction] without changing runtime behavior: - Fuse return-value conversion into single converter calls (wrap_into_ptr / wrap_into_pyobject) - Fuse required-argument unwrap+extract (extract_required_argument, from_py_with_required) and TryFrom<&Bound<T>>-style receiver extraction (extract_receiver, extract_receiver_trusted) - Fold the receiver cast into the trusted self-extraction unsafe block - Declare argument holders with a single tuple pattern and drop the vestigial per-argument Probe import - Emit PyClassImpl consts only when they differ from new trait defaults - Replace per-class compile-time doc concatenation (DOC) with DOC_PIECES assembled once at type-object creation - Outline type_object_raw into pyclass_type_object_raw helper - Share an empty PyClassItems static; skip empty impl blocks and assertion consts; drop the HasAutomaticFromPyObject deprecation probe - Shorten trampoline shim expansion; compact FunctionDescription::new
Three codegen bugs plus stale UI test snapshots:
- The fused `unsafe` extraction blocks (extract_required_argument,
from_py_with_required, extract_receiver{,_trusted}) were emitted
inside quote_spanned! with user-code spans, so the `unsafe` token
was attributed to user code and rejected by #![forbid(unsafe_code)].
Build the unsafe wrapper with call-site spans and interpolate the
spanned call into it, as the previous codegen did.
- `fn into_pyobject(...) -> PyResult<Self::Output>` is ambiguous for
enums with a variant named `Output`; spell out the concrete return
type `PyResult<Bound<'py, Self>>` instead.
- Re-bless the UI test snapshots for the renamed extraction/conversion
helpers, and update inline annotations:
- invalid_pymethod_receiver now uses a local receiver type, because
the fused receiver extraction produces a `From` candidate list for
`i32` which varies with enabled features (jiff impls under `full`)
- missing_intopy no longer emits the `map_err` error (the old
ok_wrap + map_result_into_ptr codegen is gone)
- invalid_pyclass_generic gains never_type_fallback diagnostics from
the unsafe fused helpers in already-erroneous code
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing in the macro backend emits `map_into_ptr` / `map_into_pyobject`
since return-value conversion was fused into `wrap_into_ptr` /
`wrap_into_pyobject` (`quotes::map_result_into_ptr` is gone), so drop:
- EmptyTupleConverter<PyResult<()>>::{map_into_ptr, map_into_pyobject}
- IntoPyObjectConverter<Result<T, E>>::{map_into_ptr, map_into_pyobject}
- UnknownReturnType::{map_into_ptr, map_into_pyobject} (the
unreachable! diagnostic anchors for the removed call shape)
The last reference was the example expansion in the `PyClassGuardMut`
docs, which still showed the pre-slimming codegen; update it to the
current expansion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Undo the move of class doc assembly from compile time to type-object creation time: `PyClassImpl::DOC` is back as a required const rendered at compile time via `impl_::concat`, `PyTypeBuilder::type_doc` takes the `&'static CStr` again, and the `type_doc_storage` buffer is gone. `RAW_DOC` keeps its `c""` default (part of the emit-only-non-default consts slimming), which composes the same way through `PyClassDocGenerator`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`extract_required_argument` and `from_py_with_required` are now safe functions which branch on the `Option` internally, with an `unreachable!()` arm for `None` (the macro-generated caller always provides `Some` for required arguments, previously expressed as an unsafe contract using `unreachable_unchecked`). This fixes noisy `never_type_fallback_flowing_into_unsafe` diagnostics in already-erroneous user code: when a surrounding error (e.g. an ambiguous method) leaves the extraction's type parameter unconstrained, never-type fallback kicks in, and fallback flowing into an *unsafe* call trips that deny-by-default rust-2024-compatibility lint. With a safe function the fallback is harmless and the lint does not fire, so the extra errors disappear from `invalid_pyclass_generic`. This also removes the `unsafe` block from generated argument extraction entirely (no more call-site span juggling for `#![forbid(unsafe_code)]`), and with it the now-unused `unwrap_required_argument` / `unwrap_required_argument_bound` helpers. Cost: one always-predicted branch per required argument compared to the `unreachable_unchecked` version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The removal of the `FromPyObject` blanket implementation (PyO3#6188) changed the extraction diagnostics; these snapshots are the combined output of that change and the slimmed codegen on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Headline results for pyca/cryptography:
-Zunpretty=expanded, lines)-Ztime-passes, non-incr.)Origin of these wins:
__pymethod_*wrappersPyClassImplimplsPyMethodsitems arrays__pyfunction_*wrappersPyTypeInfoimplsIntoPyObjectimpls(Work performed with Claude)