Skip to content

Commit d58a449

Browse files
davidhewittdavidhewitt
authored
refactor #[new] code generation (#5551)
* refactor `#[new]` code generation * correct merge conflicts * reverse some of the changes to method.rs * clippy * fixup introspection on `__new__` * clean up `SlotDef` and `SlotFragmentDef` construction
1 parent f64557c commit d58a449

File tree

6 files changed

+334
-316
lines changed

6 files changed

+334
-316
lines changed

‎pyo3-macros-backend/src/method.rs‎

Lines changed: 19 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,6 @@ pub enum FnType {
252252
Setter(SelfType),
253253
/// Represents a regular pymethod
254254
Fn(SelfType),
255-
/// Represents a pymethod annotated with `#[new]`, i.e. the `__new__` dunder.
256-
FnNew,
257-
/// Represents a pymethod annotated with both `#[new]` and `#[classmethod]` (in either order)
258-
FnNewClass(Span),
259255
/// Represents a pymethod annotated with `#[classmethod]`, like a `@classmethod`
260256
FnClass(Span),
261257
/// Represents a pyfunction or a pymethod annotated with `#[staticmethod]`, like a `@staticmethod`
@@ -273,20 +269,14 @@ impl FnType {
273269
| FnType::Setter(_)
274270
| FnType::Fn(_)
275271
| FnType::FnClass(_)
276-
| FnType::FnNewClass(_)
277272
| FnType::FnModule(_) => true,
278-
FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => false,
273+
FnType::FnStatic | FnType::ClassAttribute => false,
279274
}
280275
}
281276

282277
pub fn signature_attribute_allowed(&self) -> bool {
283278
match self {
284-
FnType::Fn(_)
285-
| FnType::FnNew
286-
| FnType::FnStatic
287-
| FnType::FnClass(_)
288-
| FnType::FnNewClass(_)
289-
| FnType::FnModule(_) => true,
279+
FnType::Fn(_) | FnType::FnStatic | FnType::FnClass(_) | FnType::FnModule(_) => true,
290280
// Setter, Getter and ClassAttribute all have fixed signatures (either take 0 or 1
291281
// arguments) so cannot have a `signature = (...)` attribute.
292282
FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => false,
@@ -312,7 +302,7 @@ impl FnType {
312302
syn::Token![,](Span::call_site()).to_tokens(&mut receiver);
313303
Some(receiver)
314304
}
315-
FnType::FnClass(span) | FnType::FnNewClass(span) => {
305+
FnType::FnClass(span) => {
316306
let py = syn::Ident::new("py", Span::call_site());
317307
let slf: Ident = syn::Ident::new("_slf", Span::call_site());
318308
let pyo3_path = pyo3_path.to_tokens_spanned(*span);
@@ -338,7 +328,7 @@ impl FnType {
338328
};
339329
Some(quote! { unsafe { #ret }, })
340330
}
341-
FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => None,
331+
FnType::FnStatic | FnType::ClassAttribute => None,
342332
}
343333
}
344334
}
@@ -424,12 +414,11 @@ impl SelfType {
424414
}
425415

426416
/// Determines which CPython calling convention a given FnSpec uses.
427-
#[derive(Clone, Debug)]
417+
#[derive(Clone, Debug, Copy)]
428418
pub enum CallingConvention {
429419
Noargs, // METH_NOARGS
430420
Varargs, // METH_VARARGS | METH_KEYWORDS
431421
Fastcall, // METH_FASTCALL | METH_KEYWORDS (not compatible with `abi3` feature before 3.10)
432-
TpNew, // special convention for tp_new
433422
}
434423

435424
impl CallingConvention {
@@ -461,7 +450,6 @@ pub struct FnSpec<'a> {
461450
// r# can be removed by syn::ext::IdentExt::unraw()
462451
pub python_name: syn::Ident,
463452
pub signature: FunctionSignature<'a>,
464-
pub convention: CallingConvention,
465453
pub text_signature: Option<TextSignatureAttribute>,
466454
pub asyncness: Option<syn::Token![async]>,
467455
pub unsafety: Option<syn::Token![unsafe]>,
@@ -533,16 +521,9 @@ impl<'a> FnSpec<'a> {
533521
FunctionSignature::from_arguments(arguments)
534522
};
535523

536-
let convention = if matches!(fn_type, FnType::FnNew | FnType::FnNewClass(_)) {
537-
CallingConvention::TpNew
538-
} else {
539-
CallingConvention::from_signature(&signature)
540-
};
541-
542524
Ok(FnSpec {
543525
tp: fn_type,
544526
name,
545-
convention,
546527
python_name,
547528
signature,
548529
text_signature,
@@ -600,12 +581,12 @@ impl<'a> FnSpec<'a> {
600581
[MethodTypeAttribute::ClassAttribute(_)] => FnType::ClassAttribute,
601582
[MethodTypeAttribute::New(_)] => {
602583
set_name_to_new()?;
603-
FnType::FnNew
584+
FnType::FnStatic
604585
}
605586
[MethodTypeAttribute::New(_), MethodTypeAttribute::ClassMethod(span)]
606587
| [MethodTypeAttribute::ClassMethod(span), MethodTypeAttribute::New(_)] => {
607588
set_name_to_new()?;
608-
FnType::FnNewClass(*span)
589+
FnType::FnClass(*span)
609590
}
610591
[MethodTypeAttribute::ClassMethod(_)] => {
611592
// Add a helpful hint if the classmethod doesn't look like a classmethod
@@ -677,6 +658,7 @@ impl<'a> FnSpec<'a> {
677658
&self,
678659
ident: &proc_macro2::Ident,
679660
cls: Option<&syn::Type>,
661+
convention: CallingConvention,
680662
ctx: &Ctx,
681663
) -> Result<TokenStream> {
682664
let Ctx {
@@ -799,7 +781,7 @@ impl<'a> FnSpec<'a> {
799781

800782
let warnings = self.warnings.build_py_warning(ctx);
801783

802-
Ok(match self.convention {
784+
Ok(match convention {
803785
CallingConvention::Noargs => {
804786
let mut holders = Holders::new();
805787
let args = self
@@ -872,52 +854,31 @@ impl<'a> FnSpec<'a> {
872854
}
873855
}
874856
}
875-
CallingConvention::TpNew => {
876-
let mut holders = Holders::new();
877-
let (arg_convert, args) = impl_arg_params(self, cls, false, &mut holders, ctx);
878-
let self_arg = self
879-
.tp
880-
.self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx);
881-
let call = quote_spanned! {*output_span=> #rust_name(#self_arg #(#args),*) };
882-
let init_holders = holders.init_holders(ctx);
883-
quote! {
884-
unsafe fn #ident(
885-
py: #pyo3_path::Python<'_>,
886-
_slf: *mut #pyo3_path::ffi::PyTypeObject,
887-
_args: *mut #pyo3_path::ffi::PyObject,
888-
_kwargs: *mut #pyo3_path::ffi::PyObject
889-
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
890-
use #pyo3_path::impl_::callback::IntoPyCallbackOutput;
891-
let function = #rust_name; // Shadow the function name to avoid #3017
892-
#arg_convert
893-
#init_holders
894-
#warnings
895-
let result = #call;
896-
let initializer: #pyo3_path::PyClassInitializer::<#cls> = result.convert(py)?;
897-
#pyo3_path::impl_::pymethods::tp_new_impl(py, initializer, _slf)
898-
}
899-
}
900-
}
901857
})
902858
}
903859

904860
/// Return a `PyMethodDef` constructor for this function, matching the selected
905861
/// calling convention.
906-
pub fn get_methoddef(&self, wrapper: impl ToTokens, doc: &PythonDoc, ctx: &Ctx) -> TokenStream {
862+
pub fn get_methoddef(
863+
&self,
864+
wrapper: impl ToTokens,
865+
doc: &PythonDoc,
866+
convention: CallingConvention,
867+
ctx: &Ctx,
868+
) -> TokenStream {
907869
let Ctx { pyo3_path, .. } = ctx;
908870
let python_name = self.null_terminated_python_name();
909871
let flags = match self.tp {
910872
FnType::FnClass(_) => quote! { .flags(#pyo3_path::ffi::METH_CLASS) },
911873
FnType::FnStatic => quote! { .flags(#pyo3_path::ffi::METH_STATIC) },
912874
_ => quote! {},
913875
};
914-
let trampoline = match self.convention {
876+
let trampoline = match convention {
915877
CallingConvention::Noargs => Ident::new("noargs", Span::call_site()),
916878
CallingConvention::Fastcall => {
917879
Ident::new("fastcall_cfunction_with_keywords", Span::call_site())
918880
}
919881
CallingConvention::Varargs => Ident::new("cfunction_with_keywords", Span::call_site()),
920-
CallingConvention::TpNew => unreachable!("tp_new cannot get a methoddef"),
921882
};
922883
quote! {
923884
#pyo3_path::impl_::pymethods::PyMethodDef::#trampoline(
@@ -944,8 +905,8 @@ impl<'a> FnSpec<'a> {
944905
FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => return None,
945906
FnType::Fn(_) => Some("self"),
946907
FnType::FnModule(_) => Some("module"),
947-
FnType::FnClass(_) | FnType::FnNewClass(_) => Some("cls"),
948-
FnType::FnStatic | FnType::FnNew => None,
908+
FnType::FnClass(_) => Some("cls"),
909+
FnType::FnStatic => None,
949910
};
950911

951912
match self.text_signature.as_ref().map(|attr| &attr.value) {

‎pyo3-macros-backend/src/pyclass.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use crate::pyimpl::{gen_py_const, get_cfg_attributes, PyClassMethodsType};
2828
use crate::pymethod::field_python_name;
2929
use crate::pymethod::{
3030
impl_py_class_attribute, impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef,
31-
MethodAndSlotDef, PropertyType, SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __REPR__,
32-
__RICHCMP__, __STR__,
31+
MethodAndSlotDef, PropertyType, SlotDef, __GETITEM__, __HASH__, __INT__, __LEN__, __NEW__,
32+
__REPR__, __RICHCMP__, __STR__,
3333
};
3434
use crate::pyversions::{is_abi3_before, is_py_before};
3535
use crate::utils::{self, apply_renaming_rule, Ctx, PythonDoc};
@@ -1746,11 +1746,10 @@ fn complex_enum_struct_variant_new<'a>(
17461746
};
17471747

17481748
let spec = FnSpec {
1749-
tp: crate::method::FnType::FnNew,
1749+
tp: crate::method::FnType::FnStatic,
17501750
name: &format_ident!("__pymethod_constructor__"),
17511751
python_name: format_ident!("__new__"),
17521752
signature,
1753-
convention: crate::method::CallingConvention::TpNew,
17541753
text_signature: None,
17551754
asyncness: None,
17561755
unsafety: None,
@@ -1759,7 +1758,7 @@ fn complex_enum_struct_variant_new<'a>(
17591758
output: syn::ReturnType::Default,
17601759
};
17611760

1762-
crate::pymethod::impl_py_method_def_new(&variant_cls_type, &spec, ctx)
1761+
__NEW__.generate_type_slot(&variant_cls_type, &spec, "__default___new____", ctx)
17631762
}
17641763

17651764
fn complex_enum_tuple_variant_new<'a>(
@@ -1805,11 +1804,10 @@ fn complex_enum_tuple_variant_new<'a>(
18051804
};
18061805

18071806
let spec = FnSpec {
1808-
tp: crate::method::FnType::FnNew,
1807+
tp: crate::method::FnType::FnStatic,
18091808
name: &format_ident!("__pymethod_constructor__"),
18101809
python_name: format_ident!("__new__"),
18111810
signature,
1812-
convention: crate::method::CallingConvention::TpNew,
18131811
text_signature: None,
18141812
asyncness: None,
18151813
unsafety: None,
@@ -1818,7 +1816,7 @@ fn complex_enum_tuple_variant_new<'a>(
18181816
output: syn::ReturnType::Default,
18191817
};
18201818

1821-
crate::pymethod::impl_py_method_def_new(&variant_cls_type, &spec, ctx)
1819+
__NEW__.generate_type_slot(&variant_cls_type, &spec, "__default___new____", ctx)
18221820
}
18231821

18241822
fn complex_enum_variant_field_getter<'a>(
@@ -1838,7 +1836,6 @@ fn complex_enum_variant_field_getter<'a>(
18381836
name: field_name,
18391837
python_name: field_name.unraw(),
18401838
signature,
1841-
convention: crate::method::CallingConvention::Noargs,
18421839
text_signature: None,
18431840
asyncness: None,
18441841
unsafety: None,

‎pyo3-macros-backend/src/pyfunction.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,6 @@ pub fn impl_wrap_pyfunction(
408408
let spec = method::FnSpec {
409409
tp,
410410
name: &func.sig.ident,
411-
convention: CallingConvention::from_signature(&signature),
412411
python_name,
413412
signature,
414413
text_signature,
@@ -426,8 +425,14 @@ pub fn impl_wrap_pyfunction(
426425
spec.asyncness.span() => "async functions are only supported with the `experimental-async` feature"
427426
);
428427
}
429-
let wrapper = spec.get_wrapper_function(&wrapper_ident, None, ctx)?;
430-
let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs, ctx)?, ctx);
428+
let calling_convention = CallingConvention::from_signature(&spec.signature);
429+
let wrapper = spec.get_wrapper_function(&wrapper_ident, None, calling_convention, ctx)?;
430+
let methoddef = spec.get_methoddef(
431+
wrapper_ident,
432+
&spec.get_doc(&func.attrs, ctx)?,
433+
calling_convention,
434+
ctx,
435+
);
431436

432437
let wrapped_pyfunction = quote! {
433438
// Create a module with the same name as the `#[pyfunction]` - this way `use <the function>`

‎pyo3-macros-backend/src/pyimpl.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,6 @@ fn method_introspection_code(spec: &FnSpec<'_>, parent: &syn::Type, ctx: &Ctx) -
397397

398398
// We introduce self/cls argument and setup decorators
399399
let mut first_argument = None;
400-
let mut output = spec.output.clone();
401400
let mut decorators = Vec::new();
402401
match &spec.tp {
403402
FnType::Getter(_) => {
@@ -411,16 +410,20 @@ fn method_introspection_code(spec: &FnSpec<'_>, parent: &syn::Type, ctx: &Ctx) -
411410
FnType::Fn(_) => {
412411
first_argument = Some("self");
413412
}
414-
FnType::FnNew | FnType::FnNewClass(_) => {
415-
first_argument = Some("cls");
416-
output = parse_quote!(-> #pyo3_path::PyRef<Self>); // Hack to return Self while implementing IntoPyObject
417-
}
418413
FnType::FnClass(_) => {
419414
first_argument = Some("cls");
420-
decorators.push(PythonIdentifier::builtins("classmethod"));
415+
if spec.python_name != "__new__" {
416+
// special case __new__ - does not get the decorator
417+
decorators.push(PythonIdentifier::builtins("classmethod"));
418+
}
421419
}
422420
FnType::FnStatic => {
423-
decorators.push(PythonIdentifier::builtins("staticmethod"));
421+
if spec.python_name != "__new__" {
422+
decorators.push(PythonIdentifier::builtins("staticmethod"));
423+
} else {
424+
// special case __new__ - does not get the decorator and gets first argument
425+
first_argument = Some("cls");
426+
}
424427
}
425428
FnType::FnModule(_) => (), // TODO: not sure this can happen
426429
FnType::ClassAttribute => {
@@ -430,13 +433,19 @@ fn method_introspection_code(spec: &FnSpec<'_>, parent: &syn::Type, ctx: &Ctx) -
430433
decorators.push(PythonIdentifier::builtins("property"));
431434
}
432435
}
436+
let return_type = if spec.python_name == "__new__" {
437+
// Hack to return Self while implementing IntoPyObject
438+
parse_quote!(-> #pyo3_path::PyRef<Self>)
439+
} else {
440+
spec.output.clone()
441+
};
433442
function_introspection_code(
434443
pyo3_path,
435444
None,
436445
&name,
437446
&spec.signature,
438447
first_argument,
439-
output,
448+
return_type,
440449
decorators,
441450
Some(parent),
442451
)

0 commit comments

Comments
 (0)