Skip to content

Commit d36a2cf

Browse files
youknowoneyouknowone
authored
New subclass payload layout (RustPython#6319)
* PySubclass * Add base payload to exception * PyStructSequecne base * redefine PyOSError and subtypes * heap exception new * rework UNSUPPORTED_OPERATION
1 parent 21300f6 commit d36a2cf

27 files changed

Lines changed: 1063 additions & 491 deletions

File tree

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

Lines changed: 163 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,85 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul
305305
Ok(tokens)
306306
}
307307

308+
/// Validates that when a base class is specified, the struct has the base type as its first field.
309+
/// This ensures proper memory layout for subclassing (required for #[repr(transparent)] to work correctly).
310+
fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result<()> {
311+
let Item::Struct(item_struct) = item else {
312+
// Only validate structs - enums with base are already an error elsewhere
313+
return Ok(());
314+
};
315+
316+
// Get the base type name for error messages
317+
let base_name = base_path
318+
.segments
319+
.last()
320+
.map(|s| s.ident.to_string())
321+
.unwrap_or_else(|| quote!(#base_path).to_string());
322+
323+
match &item_struct.fields {
324+
syn::Fields::Named(fields) => {
325+
let Some(first_field) = fields.named.first() else {
326+
bail_span!(
327+
item_struct,
328+
"#[pyclass] with base = {base_name} requires the first field to be of type {base_name}, but the struct has no fields"
329+
);
330+
};
331+
if !type_matches_path(&first_field.ty, base_path) {
332+
bail_span!(
333+
first_field,
334+
"#[pyclass] with base = {base_name} requires the first field to be of type {base_name}"
335+
);
336+
}
337+
}
338+
syn::Fields::Unnamed(fields) => {
339+
let Some(first_field) = fields.unnamed.first() else {
340+
bail_span!(
341+
item_struct,
342+
"#[pyclass] with base = {base_name} requires the first field to be of type {base_name}, but the struct has no fields"
343+
);
344+
};
345+
if !type_matches_path(&first_field.ty, base_path) {
346+
bail_span!(
347+
first_field,
348+
"#[pyclass] with base = {base_name} requires the first field to be of type {base_name}"
349+
);
350+
}
351+
}
352+
syn::Fields::Unit => {
353+
bail_span!(
354+
item_struct,
355+
"#[pyclass] with base = {base_name} requires the first field to be of type {base_name}, but the struct is a unit struct"
356+
);
357+
}
358+
}
359+
360+
Ok(())
361+
}
362+
363+
/// Check if a type matches a given path (handles simple cases like `Foo` or `path::to::Foo`)
364+
fn type_matches_path(ty: &syn::Type, path: &syn::Path) -> bool {
365+
// Compare by converting both to string representation for macro hygiene
366+
let ty_str = quote!(#ty).to_string().replace(' ', "");
367+
let path_str = quote!(#path).to_string().replace(' ', "");
368+
369+
// Check if both are the same or if the type ends with the path's last segment
370+
if ty_str == path_str {
371+
return true;
372+
}
373+
374+
// Also match if just the last segment matches (e.g., foo::Bar matches Bar)
375+
let syn::Type::Path(type_path) = ty else {
376+
return false;
377+
};
378+
let Some(type_last) = type_path.path.segments.last() else {
379+
return false;
380+
};
381+
let Some(path_last) = path.segments.last() else {
382+
return false;
383+
};
384+
type_last.ident == path_last.ident
385+
}
386+
308387
fn generate_class_def(
309388
ident: &Ident,
310389
name: &str,
@@ -339,7 +418,6 @@ fn generate_class_def(
339418
} else {
340419
quote!(false)
341420
};
342-
let basicsize = quote!(std::mem::size_of::<#ident>());
343421
let is_pystruct = attrs.iter().any(|attr| {
344422
attr.path().is_ident("derive")
345423
&& if let Ok(Meta::List(l)) = attr.parse_meta() {
@@ -350,6 +428,25 @@ fn generate_class_def(
350428
false
351429
}
352430
});
431+
// Check if the type has #[repr(transparent)] - only then we can safely
432+
// generate PySubclass impl (requires same memory layout as base type)
433+
let is_repr_transparent = attrs.iter().any(|attr| {
434+
attr.path().is_ident("repr")
435+
&& if let Ok(Meta::List(l)) = attr.parse_meta() {
436+
l.nested
437+
.into_iter()
438+
.any(|n| n.get_ident().is_some_and(|p| p == "transparent"))
439+
} else {
440+
false
441+
}
442+
});
443+
// If repr(transparent) with a base, the type has the same memory layout as base,
444+
// so basicsize should be 0 (no additional space beyond the base type)
445+
let basicsize = if is_repr_transparent && base.is_some() {
446+
quote!(0)
447+
} else {
448+
quote!(std::mem::size_of::<#ident>())
449+
};
353450
if base.is_some() && is_pystruct {
354451
bail_span!(ident, "PyStructSequence cannot have `base` class attr",);
355452
}
@@ -379,12 +476,31 @@ fn generate_class_def(
379476
}
380477
});
381478

382-
let base_or_object = if let Some(base) = base {
479+
let base_or_object = if let Some(ref base) = base {
383480
quote! { #base }
384481
} else {
385482
quote! { ::rustpython_vm::builtins::PyBaseObject }
386483
};
387484

485+
// Generate PySubclass impl for #[repr(transparent)] types with base class
486+
// (tuple struct assumed, so &self.0 works)
487+
let subclass_impl = if !is_pystruct && is_repr_transparent {
488+
base.as_ref().map(|typ| {
489+
quote! {
490+
impl ::rustpython_vm::class::PySubclass for #ident {
491+
type Base = #typ;
492+
493+
#[inline]
494+
fn as_base(&self) -> &Self::Base {
495+
&self.0
496+
}
497+
}
498+
}
499+
})
500+
} else {
501+
None
502+
};
503+
388504
let tokens = quote! {
389505
impl ::rustpython_vm::class::PyClassDef for #ident {
390506
const NAME: &'static str = #name;
@@ -409,6 +525,8 @@ fn generate_class_def(
409525

410526
#base_class
411527
}
528+
529+
#subclass_impl
412530
};
413531
Ok(tokens)
414532
}
@@ -426,11 +544,16 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result<Tok
426544
let metaclass = class_meta.metaclass()?;
427545
let unhashable = class_meta.unhashable()?;
428546

547+
// Validate that if base is specified, the first field must be of the base type
548+
if let Some(ref base_path) = base {
549+
validate_base_field(&item, base_path)?;
550+
}
551+
429552
let class_def = generate_class_def(
430553
ident,
431554
&class_name,
432555
module_name.as_deref(),
433-
base,
556+
base.clone(),
434557
metaclass,
435558
unhashable,
436559
attrs,
@@ -485,19 +608,47 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result<Tok
485608
}
486609
};
487610

488-
let impl_payload = if let Some(ctx_type_name) = class_meta.ctx_name()? {
489-
let ctx_type_ident = Ident::new(&ctx_type_name, ident.span()); // FIXME span
611+
// Generate PyPayload impl based on whether base exists
612+
#[allow(clippy::collapsible_else_if)]
613+
let impl_payload = if let Some(base_type) = &base {
614+
let class_fn = if let Some(ctx_type_name) = class_meta.ctx_name()? {
615+
let ctx_type_ident = Ident::new(&ctx_type_name, ident.span());
616+
quote! { ctx.types.#ctx_type_ident }
617+
} else {
618+
quote! { <Self as ::rustpython_vm::class::StaticType>::static_type() }
619+
};
490620

491-
// We need this to make extend mechanism work:
492621
quote! {
622+
// static_assertions::const_assert!(std::mem::size_of::<#base_type>() <= std::mem::size_of::<#ident>());
493623
impl ::rustpython_vm::PyPayload for #ident {
624+
#[inline]
625+
fn payload_type_id() -> ::std::any::TypeId {
626+
<#base_type as ::rustpython_vm::PyPayload>::payload_type_id()
627+
}
628+
629+
#[inline]
630+
fn validate_downcastable_from(obj: &::rustpython_vm::PyObject) -> bool {
631+
<Self as ::rustpython_vm::class::PyClassDef>::BASICSIZE <= obj.class().slots.basicsize && obj.class().fast_issubclass(<Self as ::rustpython_vm::class::StaticType>::static_type())
632+
}
633+
494634
fn class(ctx: &::rustpython_vm::vm::Context) -> &'static ::rustpython_vm::Py<::rustpython_vm::builtins::PyType> {
495-
ctx.types.#ctx_type_ident
635+
#class_fn
496636
}
497637
}
498638
}
499639
} else {
500-
quote! {}
640+
if let Some(ctx_type_name) = class_meta.ctx_name()? {
641+
let ctx_type_ident = Ident::new(&ctx_type_name, ident.span());
642+
quote! {
643+
impl ::rustpython_vm::PyPayload for #ident {
644+
fn class(ctx: &::rustpython_vm::vm::Context) -> &'static ::rustpython_vm::Py<::rustpython_vm::builtins::PyType> {
645+
ctx.types.#ctx_type_ident
646+
}
647+
}
648+
}
649+
} else {
650+
quote! {}
651+
}
501652
};
502653

503654
let empty_impl = if let Some(attrs) = class_meta.impl_attrs()? {
@@ -536,26 +687,6 @@ pub(crate) fn impl_pyexception(attr: PunctuatedNestedMeta, item: Item) -> Result
536687
let class_name = class_meta.class_name()?;
537688

538689
let base_class_name = class_meta.base()?;
539-
let impl_payload = if let Some(ctx_type_name) = class_meta.ctx_name()? {
540-
let ctx_type_ident = Ident::new(&ctx_type_name, ident.span()); // FIXME span
541-
542-
// We need this to make extend mechanism work:
543-
quote! {
544-
impl ::rustpython_vm::PyPayload for #ident {
545-
fn class(ctx: &::rustpython_vm::vm::Context) -> &'static ::rustpython_vm::Py<::rustpython_vm::builtins::PyType> {
546-
ctx.exceptions.#ctx_type_ident
547-
}
548-
}
549-
}
550-
} else {
551-
quote! {
552-
impl ::rustpython_vm::PyPayload for #ident {
553-
fn class(_ctx: &::rustpython_vm::vm::Context) -> &'static ::rustpython_vm::Py<::rustpython_vm::builtins::PyType> {
554-
<Self as ::rustpython_vm::class::StaticType>::static_type()
555-
}
556-
}
557-
}
558-
};
559690
let impl_pyclass = if class_meta.has_impl()? {
560691
quote! {
561692
#[pyexception]
@@ -568,7 +699,6 @@ pub(crate) fn impl_pyexception(attr: PunctuatedNestedMeta, item: Item) -> Result
568699
let ret = quote! {
569700
#[pyclass(module = false, name = #class_name, base = #base_class_name)]
570701
#item
571-
#impl_payload
572702
#impl_pyclass
573703
};
574704
Ok(ret)
@@ -585,7 +715,8 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
585715
let mut extra_attrs = Vec::new();
586716
for nested in &attr {
587717
if let NestedMeta::Meta(Meta::List(MetaList { path, nested, .. })) = nested {
588-
if path.is_ident("with") {
718+
// If we already found the constructor trait, no need to keep looking for it
719+
if !has_slot_new && path.is_ident("with") {
589720
// Check if Constructor is in the list
590721
for meta in nested {
591722
if let NestedMeta::Meta(Meta::Path(p)) = meta
@@ -1078,9 +1209,8 @@ impl GetSetNursery {
10781209
item_ident: Ident,
10791210
) -> Result<()> {
10801211
assert!(!self.validated, "new item is not allowed after validation");
1081-
if !matches!(kind, GetSetItemKind::Get) && !cfgs.is_empty() {
1082-
bail_span!(item_ident, "Only the getter can have #[cfg]",);
1083-
}
1212+
// Note: Both getter and setter can have #[cfg], but they must have matching cfgs
1213+
// since the map key is (name, cfgs). This ensures getter and setter are paired correctly.
10841214
let entry = self.map.entry((name.clone(), cfgs)).or_default();
10851215
let func = match kind {
10861216
GetSetItemKind::Get => &mut entry.0,

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -446,8 +446,10 @@ pub(crate) fn impl_pystruct_sequence(
446446
};
447447

448448
let output = quote! {
449-
// The Python type struct (user-defined, possibly empty)
450-
#pytype_vis struct #pytype_ident;
449+
// The Python type struct - newtype wrapping PyTuple
450+
#[derive(Debug)]
451+
#[repr(transparent)]
452+
#pytype_vis struct #pytype_ident(pub ::rustpython_vm::builtins::PyTuple);
451453

452454
// PyClassDef for Python type
453455
impl ::rustpython_vm::class::PyClassDef for #pytype_ident {
@@ -476,10 +478,37 @@ pub(crate) fn impl_pystruct_sequence(
476478
}
477479
}
478480

479-
// MaybeTraverse (empty - no GC fields in empty struct)
481+
// Subtype uses base type's payload_type_id
482+
impl ::rustpython_vm::PyPayload for #pytype_ident {
483+
#[inline]
484+
fn payload_type_id() -> ::std::any::TypeId {
485+
<::rustpython_vm::builtins::PyTuple as ::rustpython_vm::PyPayload>::payload_type_id()
486+
}
487+
488+
#[inline]
489+
fn validate_downcastable_from(obj: &::rustpython_vm::PyObject) -> bool {
490+
obj.class().fast_issubclass(<Self as ::rustpython_vm::class::StaticType>::static_type())
491+
}
492+
493+
fn class(_ctx: &::rustpython_vm::vm::Context) -> &'static ::rustpython_vm::Py<::rustpython_vm::builtins::PyType> {
494+
<Self as ::rustpython_vm::class::StaticType>::static_type()
495+
}
496+
}
497+
498+
// MaybeTraverse - delegate to inner PyTuple
480499
impl ::rustpython_vm::object::MaybeTraverse for #pytype_ident {
481-
fn try_traverse(&self, _traverse_fn: &mut ::rustpython_vm::object::TraverseFn<'_>) {
482-
// Empty struct has no fields to traverse
500+
fn try_traverse(&self, traverse_fn: &mut ::rustpython_vm::object::TraverseFn<'_>) {
501+
self.0.try_traverse(traverse_fn)
502+
}
503+
}
504+
505+
// PySubclass for proper inheritance
506+
impl ::rustpython_vm::class::PySubclass for #pytype_ident {
507+
type Base = ::rustpython_vm::builtins::PyTuple;
508+
509+
#[inline]
510+
fn as_base(&self) -> &Self::Base {
511+
&self.0
483512
}
484513
}
485514

‎crates/stdlib/src/socket.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ mod _socket {
1515
use crate::common::lock::{PyMappedRwLockReadGuard, PyRwLock, PyRwLockReadGuard};
1616
use crate::vm::{
1717
AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
18-
builtins::{PyBaseExceptionRef, PyListRef, PyStrRef, PyTupleRef, PyTypeRef},
18+
builtins::{PyBaseExceptionRef, PyListRef, PyOSError, PyStrRef, PyTupleRef, PyTypeRef},
1919
common::os::ErrorExt,
2020
convert::{IntoPyException, ToPyObject, TryFromBorrowedObject, TryFromObject},
2121
function::{ArgBytesLike, ArgMemoryBuffer, Either, FsPath, OptionalArg, OptionalOption},
@@ -1826,6 +1826,11 @@ mod _socket {
18261826
Self::Py(exc)
18271827
}
18281828
}
1829+
impl From<PyRef<PyOSError>> for IoOrPyException {
1830+
fn from(exc: PyRef<PyOSError>) -> Self {
1831+
Self::Py(exc.upcast())
1832+
}
1833+
}
18291834
impl From<io::Error> for IoOrPyException {
18301835
fn from(err: io::Error) -> Self {
18311836
Self::Io(err)
@@ -1844,7 +1849,7 @@ mod _socket {
18441849
#[inline]
18451850
fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef {
18461851
match self {
1847-
Self::Timeout => timeout_error(vm),
1852+
Self::Timeout => timeout_error(vm).upcast(),
18481853
Self::Py(exc) => exc,
18491854
Self::Io(err) => err.into_pyexception(vm),
18501855
}
@@ -2412,18 +2417,15 @@ mod _socket {
24122417
SocketError::GaiError => gaierror(vm),
24132418
SocketError::HError => herror(vm),
24142419
};
2415-
vm.new_exception(
2416-
exception_cls,
2417-
vec![vm.new_pyobj(err.error_num()), vm.ctx.new_str(strerr).into()],
2418-
)
2419-
.into()
2420+
vm.new_os_subtype_error(exception_cls, Some(err.error_num()), strerr)
2421+
.into()
24202422
}
24212423

2422-
fn timeout_error(vm: &VirtualMachine) -> PyBaseExceptionRef {
2424+
fn timeout_error(vm: &VirtualMachine) -> PyRef<PyOSError> {
24232425
timeout_error_msg(vm, "timed out".to_owned())
24242426
}
2425-
pub(crate) fn timeout_error_msg(vm: &VirtualMachine, msg: String) -> PyBaseExceptionRef {
2426-
vm.new_exception_msg(timeout(vm), msg)
2427+
pub(crate) fn timeout_error_msg(vm: &VirtualMachine, msg: String) -> PyRef<PyOSError> {
2428+
vm.new_os_subtype_error(timeout(vm), None, msg)
24272429
}
24282430

24292431
fn get_ipv6_addr_str(ipv6: Ipv6Addr) -> String {

0 commit comments

Comments
 (0)