TypeData by youknowone · Pull Request #6403 · RustPython/RustPython

youknowone

95-123: Avoid unwrap() on downcast_ref and use StgInfo alignment instead of size in __mul__ / meta in_dll

Both __mul__ and the metaclass-level in_dll currently do:

let type_ref = zelf.downcast_ref::<PyType>().unwrap();

If zelf is ever not a PyType (e.g. misused from Python or via an internal call), this will panic instead of raising a Python exception. Past review already pointed this out for both call sites; the issue is still present.

Additionally, __mul__ passes the inner size as the align argument to StgInfo::new_array:

let (_inner_length, inner_size) = ... stg.size ...
...
let stg_info = StgInfo::new_array(
    total_size,
    new_element_size, // <-- using size as align
    n as usize,
    current_array_type,
    new_element_size,
);

Conceptually, array alignment should come from the inner type’s alignment (stg.align), not from its total size. Using size as align will give incorrect alignment for nested arrays and any place that relies on align (e.g. libffi layouts).

A concrete fix that addresses both concerns and makes missing StgInfo a Python error could look like:

@@
     #[pymethod]
     fn __mul__(zelf: PyObjectRef, n: isize, vm: &VirtualMachine) -> PyResult {
         if n < 0 {
             return Err(vm.new_value_error(format!("Array length must be >= 0, not {n}")));
         }

-        // Get inner array info from TypeDataSlot
-        let type_ref = zelf.downcast_ref::<PyType>().unwrap();
-        let (_inner_length, inner_size) = type_ref
-            .get_type_data::<StgInfo>()
-            .map(|stg| (stg.length, stg.size))
-            .unwrap_or((0, 0));
+        // Get inner array info from type_data; require `zelf` to be a type with StgInfo
+        let type_ref = zelf.downcast_ref::<PyType>().ok_or_else(|| {
+            vm.new_type_error("descriptor '__mul__' requires a type object".to_owned())
+        })?;
+        let (inner_size, inner_align) = type_ref
+            .get_type_data::<StgInfo>()
+            .map(|stg| (stg.size, stg.align))
+            .ok_or_else(|| {
+                vm.new_type_error("array type has no storage info (StgInfo)".to_owned())
+            })?;

         // The element type of the new array is the current array type itself
         let current_array_type: PyObjectRef = zelf.clone();

-        // Element size is the total size of the inner array
-        let new_element_size = inner_size;
-        let total_size = new_element_size * (n as usize);
-
-        let stg_info = StgInfo::new_array(
-            total_size,
-            new_element_size,
-            n as usize,
-            current_array_type,
-            new_element_size,
-        );
+        // Each element is the full inner array
+        let element_size = inner_size;
+        let total_size = element_size * (n as usize);
+
+        let stg_info = StgInfo::new_array(
+            total_size,
+            inner_align,
+            n as usize,
+            current_array_type,
+            element_size,
+        );

         create_array_type_with_stginfo(stg_info, vm)
@@
-        // Get size from the array type via TypeDataSlot
-        let type_ref = zelf.downcast_ref::<PyType>().unwrap();
-        let (element_type, length, element_size) = type_ref
-            .get_type_data::<StgInfo>()
-            .map(|stg| {
-                (
-                    stg.element_type.clone().unwrap_or_else(|| vm.ctx.none()),
-                    stg.length,
-                    stg.element_size,
-                )
-            })
-            .unwrap_or_else(|| (vm.ctx.none(), 0, 0));
+        // Get size from the array type via type_data
+        let type_ref = zelf.downcast_ref::<PyType>().ok_or_else(|| {
+            vm.new_type_error("in_dll requires a type object".to_owned())
+        })?;
+        let stg = type_ref
+            .get_type_data::<StgInfo>()
+            .ok_or_else(|| {
+                vm.new_type_error("array type has no storage info (StgInfo)".to_owned())
+            })?;
+        let element_type = stg.element_type.clone().unwrap_or_else(|| vm.ctx.none());
+        let length = stg.length;
+        let element_size = stg.element_size;

This keeps the normal path unchanged for correctly constructed array types (which always have a StgInfo set via create_array_type_with_stginfo), while:

eliminating potential panics from unwrap(), and
using the stored alignment as intended by StgInfo::new_array.

Also applies to: 172-184