Reduce the amount of code we generate from macros#6187
Conversation
davidhewitt
left a comment
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.
|
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>
|
@davidhewitt any blockers to landing this? |
davidhewitt
left a comment
There was a problem hiding this comment.
Sorry for the delay, no blockers other than me being busy over the past couple of weeks. Thanks for the cleanups!
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)