Skip to content

Commit fdd2ac3

Browse files
youknowoneyouknowone
authored
disallow __new__, __init__ (#6446)
* disallow __new__, __init__ * migrate to Initializer * apply review
1 parent 569bee1 commit fdd2ac3

File tree

7 files changed

+458
-369
lines changed

7 files changed

+458
-369
lines changed

‎crates/derive-impl/src/pyclass.rs‎

Lines changed: 74 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ impl FromStr for AttrName {
6363

6464
#[derive(Default)]
6565
struct ImplContext {
66+
is_trait: bool,
6667
attribute_items: ItemNursery,
6768
method_items: MethodNursery,
6869
getset_items: GetSetNursery,
@@ -232,7 +233,10 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul
232233
}
233234
}
234235
Item::Trait(mut trai) => {
235-
let mut context = ImplContext::default();
236+
let mut context = ImplContext {
237+
is_trait: true,
238+
..Default::default()
239+
};
236240
let mut has_extend_slots = false;
237241
for item in &trai.items {
238242
let has = match item {
@@ -710,21 +714,16 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
710714
};
711715

712716
// Check if with(Constructor) is specified. If Constructor trait is used, don't generate slot_new
713-
let mut has_slot_new = false;
714-
715717
let mut extra_attrs = Vec::new();
718+
let mut with_items = vec![];
716719
for nested in &attr {
717720
if let NestedMeta::Meta(Meta::List(MetaList { path, nested, .. })) = nested {
718721
// If we already found the constructor trait, no need to keep looking for it
719-
if !has_slot_new && path.is_ident("with") {
720-
// Check if Constructor is in the list
722+
if path.is_ident("with") {
721723
for meta in nested {
722-
if let NestedMeta::Meta(Meta::Path(p)) = meta
723-
&& p.is_ident("Constructor")
724-
{
725-
has_slot_new = true;
726-
}
724+
with_items.push(meta.get_ident().expect("with() has non-ident item").clone());
727725
}
726+
continue;
728727
}
729728
extra_attrs.push(NestedMeta::Meta(Meta::List(MetaList {
730729
path: path.clone(),
@@ -734,43 +733,40 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
734733
}
735734
}
736735

737-
let mut has_slot_init = false;
736+
let with_contains = |with_items: &[Ident], s: &str| {
737+
// Check if Constructor is in the list
738+
with_items.iter().any(|ident| ident == s)
739+
};
740+
738741
let syn::ItemImpl {
739742
generics,
740743
self_ty,
741744
items,
742745
..
743746
} = &imp;
744-
for item in items {
745-
// FIXME: better detection or correct wrapper implementation
746-
let Some(ident) = item.get_ident() else {
747-
continue;
748-
};
749-
let item_name = ident.to_string();
750-
match item_name.as_str() {
751-
"slot_new" => {
752-
has_slot_new = true;
753-
}
754-
"slot_init" => {
755-
has_slot_init = true;
756-
}
757-
_ => continue,
758-
}
759-
}
760-
761-
// TODO: slot_new, slot_init must be Constructor or Initializer later
762747

763-
let slot_new = if has_slot_new {
748+
let slot_new = if with_contains(&with_items, "Constructor") {
764749
quote!()
765750
} else {
751+
with_items.push(Ident::new("Constructor", Span::call_site()));
766752
quote! {
767-
#[pyslot]
768-
pub fn slot_new(
769-
cls: ::rustpython_vm::builtins::PyTypeRef,
770-
args: ::rustpython_vm::function::FuncArgs,
771-
vm: &::rustpython_vm::VirtualMachine,
772-
) -> ::rustpython_vm::PyResult {
773-
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_new(cls, args, vm)
753+
impl ::rustpython_vm::types::Constructor for #self_ty {
754+
type Args = ::rustpython_vm::function::FuncArgs;
755+
756+
fn slot_new(
757+
cls: ::rustpython_vm::builtins::PyTypeRef,
758+
args: ::rustpython_vm::function::FuncArgs,
759+
vm: &::rustpython_vm::VirtualMachine,
760+
) -> ::rustpython_vm::PyResult {
761+
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_new(cls, args, vm)
762+
}
763+
fn py_new(
764+
_cls: &::rustpython_vm::Py<::rustpython_vm::builtins::PyType>,
765+
_args: Self::Args,
766+
_vm: &::rustpython_vm::VirtualMachine
767+
) -> ::rustpython_vm::PyResult<Self> {
768+
unreachable!("slot_new is defined")
769+
}
774770
}
775771
}
776772
};
@@ -779,19 +775,29 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
779775
// from `BaseException` in `SimpleExtendsException` macro.
780776
// See: `(initproc)BaseException_init`
781777
// spell-checker:ignore initproc
782-
let slot_init = if has_slot_init {
778+
let slot_init = if with_contains(&with_items, "Initializer") {
783779
quote!()
784780
} else {
785-
// FIXME: this is a generic logic for types not only for exceptions
781+
with_items.push(Ident::new("Initializer", Span::call_site()));
786782
quote! {
787-
#[pyslot]
788-
#[pymethod(name="__init__")]
789-
pub fn slot_init(
790-
zelf: ::rustpython_vm::PyObjectRef,
791-
args: ::rustpython_vm::function::FuncArgs,
792-
vm: &::rustpython_vm::VirtualMachine,
793-
) -> ::rustpython_vm::PyResult<()> {
794-
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_init(zelf, args, vm)
783+
impl ::rustpython_vm::types::Initializer for #self_ty {
784+
type Args = ::rustpython_vm::function::FuncArgs;
785+
786+
fn slot_init(
787+
zelf: ::rustpython_vm::PyObjectRef,
788+
args: ::rustpython_vm::function::FuncArgs,
789+
vm: &::rustpython_vm::VirtualMachine,
790+
) -> ::rustpython_vm::PyResult<()> {
791+
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_init(zelf, args, vm)
792+
}
793+
794+
fn init(
795+
_zelf: ::rustpython_vm::PyRef<Self>,
796+
_args: Self::Args,
797+
_vm: &::rustpython_vm::VirtualMachine
798+
) -> ::rustpython_vm::PyResult<()> {
799+
unreachable!("slot_init is defined")
800+
}
795801
}
796802
}
797803
};
@@ -803,13 +809,13 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
803809
};
804810

805811
Ok(quote! {
806-
#[pyclass(flags(BASETYPE, HAS_DICT) #extra_attrs_tokens)]
812+
#[pyclass(flags(BASETYPE, HAS_DICT), with(#(#with_items),*) #extra_attrs_tokens)]
807813
impl #generics #self_ty {
808814
#(#items)*
809-
810-
#slot_new
811-
#slot_init
812815
}
816+
817+
#slot_new
818+
#slot_init
813819
})
814820
}
815821

@@ -892,6 +898,23 @@ where
892898
let item_meta = MethodItemMeta::from_attr(ident.clone(), &item_attr)?;
893899

894900
let py_name = item_meta.method_name()?;
901+
902+
// Disallow __new__ and __init__ as pymethod in impl blocks (not in traits)
903+
if !args.context.is_trait {
904+
if py_name == "__new__" {
905+
return Err(syn::Error::new(
906+
ident.span(),
907+
"#[pymethod] cannot define '__new__'. Use #[pyclass(with(Constructor))] instead.",
908+
));
909+
}
910+
if py_name == "__init__" {
911+
return Err(syn::Error::new(
912+
ident.span(),
913+
"#[pymethod] cannot define '__init__'. Use #[pyclass(with(Initializer))] instead.",
914+
));
915+
}
916+
}
917+
895918
let raw = item_meta.raw()?;
896919
let sig_doc = text_signature(func.sig(), &py_name);
897920

‎crates/stdlib/src/sqlite.rs‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,7 @@ mod _sqlite {
15401540
size: Option<c_int>,
15411541
}
15421542

1543-
#[pyclass(with(Constructor, IterNext, Iterable), flags(BASETYPE))]
1543+
#[pyclass(with(Constructor, Initializer, IterNext, Iterable), flags(BASETYPE))]
15441544
impl Cursor {
15451545
fn new(
15461546
connection: PyRef<Connection>,
@@ -1571,24 +1571,6 @@ mod _sqlite {
15711571
}
15721572
}
15731573

1574-
#[pymethod]
1575-
fn __init__(&self, _connection: PyRef<Connection>, _vm: &VirtualMachine) -> PyResult<()> {
1576-
let mut guard = self.inner.lock();
1577-
if guard.is_some() {
1578-
// Already initialized (e.g., from a call to super().__init__)
1579-
return Ok(());
1580-
}
1581-
*guard = Some(CursorInner {
1582-
description: None,
1583-
row_cast_map: vec![],
1584-
lastrowid: -1,
1585-
rowcount: -1,
1586-
statement: None,
1587-
closed: false,
1588-
});
1589-
Ok(())
1590-
}
1591-
15921574
fn check_cursor_state(inner: Option<&CursorInner>, vm: &VirtualMachine) -> PyResult<()> {
15931575
match inner {
15941576
Some(inner) if inner.closed => Err(new_programming_error(
@@ -1949,6 +1931,27 @@ mod _sqlite {
19491931
}
19501932
}
19511933

1934+
impl Initializer for Cursor {
1935+
type Args = PyRef<Connection>;
1936+
1937+
fn init(zelf: PyRef<Self>, _connection: Self::Args, _vm: &VirtualMachine) -> PyResult<()> {
1938+
let mut guard = zelf.inner.lock();
1939+
if guard.is_some() {
1940+
// Already initialized (e.g., from a call to super().__init__)
1941+
return Ok(());
1942+
}
1943+
*guard = Some(CursorInner {
1944+
description: None,
1945+
row_cast_map: vec![],
1946+
lastrowid: -1,
1947+
rowcount: -1,
1948+
statement: None,
1949+
closed: false,
1950+
});
1951+
Ok(())
1952+
}
1953+
}
1954+
19521955
impl SelfIter for Cursor {}
19531956
impl IterNext for Cursor {
19541957
fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> {

‎crates/stdlib/src/ssl/error.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub(crate) mod ssl_error {
77
use crate::vm::{
88
PyPayload, PyRef, PyResult, VirtualMachine,
99
builtins::{PyBaseExceptionRef, PyOSError, PyStrRef},
10-
types::Constructor,
10+
types::{Constructor, Initializer},
1111
};
1212

1313
// Error type constants - exposed as pyattr and available for internal use

‎crates/vm/src/builtins/object.rs‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use super::{PyDictRef, PyList, PyStr, PyStrRef, PyType, PyTypeRef};
22
use crate::common::hash::PyHash;
33
use crate::types::PyTypeFlags;
44
use crate::{
5-
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
5+
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
66
class::PyClassImpl,
77
convert::ToPyResult,
88
function::{Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue},
9-
types::{Constructor, PyComparisonOp},
9+
types::{Constructor, Initializer, PyComparisonOp},
1010
};
1111
use itertools::Itertools;
1212

@@ -115,6 +115,18 @@ impl Constructor for PyBaseObject {
115115
}
116116
}
117117

118+
impl Initializer for PyBaseObject {
119+
type Args = FuncArgs;
120+
121+
fn slot_init(_zelf: PyObjectRef, _args: FuncArgs, _vm: &VirtualMachine) -> PyResult<()> {
122+
Ok(())
123+
}
124+
125+
fn init(_zelf: PyRef<Self>, _args: Self::Args, _vm: &VirtualMachine) -> PyResult<()> {
126+
unreachable!("slot_init is defined")
127+
}
128+
}
129+
118130
// TODO: implement _PyType_GetSlotNames properly
119131
fn type_slot_names(typ: &Py<PyType>, vm: &VirtualMachine) -> PyResult<Option<super::PyListRef>> {
120132
// let attributes = typ.attributes.read();
@@ -235,7 +247,7 @@ fn object_getstate_default(obj: &PyObject, required: bool, vm: &VirtualMachine)
235247
// getstate.call((), vm)
236248
// }
237249

238-
#[pyclass(with(Constructor), flags(BASETYPE))]
250+
#[pyclass(with(Constructor, Initializer), flags(BASETYPE))]
239251
impl PyBaseObject {
240252
#[pymethod(raw)]
241253
fn __getstate__(vm: &VirtualMachine, args: FuncArgs) -> PyResult {
@@ -444,19 +456,17 @@ impl PyBaseObject {
444456
obj.str(vm)
445457
}
446458

447-
#[pyslot]
448-
#[pymethod]
449-
fn __init__(_zelf: PyObjectRef, _args: FuncArgs, _vm: &VirtualMachine) -> PyResult<()> {
450-
Ok(())
451-
}
452-
453459
#[pygetset]
454460
fn __class__(obj: PyObjectRef) -> PyTypeRef {
455461
obj.class().to_owned()
456462
}
457463

458-
#[pygetset(name = "__class__", setter)]
459-
fn set_class(instance: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
464+
#[pygetset(setter)]
465+
fn set___class__(
466+
instance: PyObjectRef,
467+
value: PyObjectRef,
468+
vm: &VirtualMachine,
469+
) -> PyResult<()> {
460470
match value.downcast::<PyType>() {
461471
Ok(cls) => {
462472
let both_module = instance.class().fast_issubclass(vm.ctx.types.module_type)

0 commit comments

Comments
 (0)