Fix best_base to select proper base class (RustPython#5324)

Snowapril · web-flow · commit aa0353a501d1 · 2024-05-19T15:48:56.000+09:00
* add __basicsize__ getter

* modify best_base function to get proper base

* inherit basicsize from the base type

---------

Signed-off-by: snowapril &lt;sinjihng@gmail.com&gt;
diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py
@@ -2368,8 +2368,6 @@ def __del__(self):
 
 
 class TestType(unittest.TestCase):
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_new_type(self):
         A = type('A', (), {})
         self.assertEqual(A.__name__, 'A')
@@ -2472,8 +2470,6 @@ def test_type_doc(self):
             A.__doc__ = doc
             self.assertEqual(A.__doc__, doc)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_bad_args(self):
         with self.assertRaises(TypeError):
             type()
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
@@ -823,8 +823,6 @@ class X(C, int()):
             class X(int(), C):
                 pass
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_module_subclasses(self):
         # Testing Python subclass of module...
         log = []
@@ -1500,8 +1498,6 @@ class someclass(metaclass=dynamicmetaclass):
             pass
         self.assertNotEqual(someclass, object)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_errors(self):
         # Testing errors...
         try:
diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
@@ -1680,8 +1680,6 @@ def test_initgroups(self):
         posix.initgroups(name, g)
         self.assertIn(g, posix.getgroups())
 
-    # TODO: RUSTPYTHON: TypeError: Unexpected keyword argument setpgroup
-    @unittest.expectedFailure
     @unittest.skipUnless(hasattr(posix, 'setgroups'),
                          "test needs posix.setgroups()")
     def test_setgroups(self):
diff --git a/vm/src/builtins/type.rs b/vm/src/builtins/type.rs
@@ -205,6 +205,9 @@ impl PyType {
         if base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
             slots.flags |= PyTypeFlags::HAS_DICT
         }
+        if slots.basicsize == 0 {
+            slots.basicsize = base.slots.basicsize;
+        }
 
         if let Some(qualname) = attrs.get(identifier!(ctx, __qualname__)) {
             if !qualname.fast_isinstance(ctx.types.str_type) {
@@ -253,6 +256,9 @@ impl PyType {
         if base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
             slots.flags |= PyTypeFlags::HAS_DICT
         }
+        if slots.basicsize == 0 {
+            slots.basicsize = base.slots.basicsize;
+        }
 
         let bases = vec![base.clone()];
         let mro = base.iter_mro().map(|x| x.to_owned()).collect();
@@ -470,6 +476,11 @@ impl PyType {
         self.slots.flags.bits()
     }
 
+    #[pygetset(magic)]
+    fn basicsize(&self) -> usize {
+        self.slots.basicsize
+    }
+
     #[pygetset]
     pub fn __name__(&self, vm: &VirtualMachine) -> PyStrRef {
         self.name_inner(
@@ -1313,22 +1324,29 @@ fn calculate_meta_class(
     Ok(winner)
 }
 
+fn solid_base(typ: &PyTypeRef, vm: &VirtualMachine) -> PyTypeRef {
+    let base = if let Some(base) = &typ.base {
+        solid_base(base, vm)
+    } else {
+        vm.ctx.types.object_type.to_owned()
+    };
+
+    // TODO: itemsize comparation also needed
+    if typ.basicsize() != base.basicsize() {
+        typ.clone()
+    } else {
+        base
+    }
+}
+
 fn best_base(bases: &[PyTypeRef], vm: &VirtualMachine) -> PyResult<PyTypeRef> {
-    // let mut base = None;
-    // let mut winner = None;
+    let mut base: Option<PyTypeRef> = None;
+    let mut winner: Option<PyTypeRef> = None;
 
     for base_i in bases {
-        // base_proto = PyTuple_GET_ITEM(bases, i);
-        // if (!PyType_Check(base_proto)) {
-        //     PyErr_SetString(
-        //         PyExc_TypeError,
-        //         "bases must be types");
-        //     return NULL;
-        // }
-        // base_i = (PyTypeObject *)base_proto;
-        // if (base_i->slot_dict == NULL) {
-        //     if (PyType_Ready(base_i) < 0)
-        //         return NULL;
+        // if !base_i.fast_issubclass(vm.ctx.types.type_type) {
+        //     println!("base_i type : {}", base_i.name());
+        //     return Err(vm.new_type_error("best must be types".into()));
         // }
 
         if !base_i.slots.flags.has_feature(PyTypeFlags::BASETYPE) {
@@ -1337,28 +1355,25 @@ fn best_base(bases: &[PyTypeRef], vm: &VirtualMachine) -> PyResult<PyTypeRef> {
                 base_i.name()
             )));
         }
-        // candidate = solid_base(base_i);
-        // if (winner == NULL) {
-        //     winner = candidate;
-        //     base = base_i;
-        // }
-        // else if (PyType_IsSubtype(winner, candidate))
-        //     ;
-        // else if (PyType_IsSubtype(candidate, winner)) {
-        //     winner = candidate;
-        //     base = base_i;
-        // }
-        // else {
-        //     PyErr_SetString(
-        //         PyExc_TypeError,
-        //         "multiple bases have "
-        //         "instance lay-out conflict");
-        //     return NULL;
-        // }
+
+        let candidate = solid_base(base_i, vm);
+        if winner.is_none() {
+            winner = Some(candidate.clone());
+            base = Some(base_i.clone());
+        } else if winner.as_ref().unwrap().fast_issubclass(&candidate) {
+            // Do nothing
+        } else if candidate.fast_issubclass(winner.as_ref().unwrap()) {
+            winner = Some(candidate.clone());
+            base = Some(base_i.clone());
+        } else {
+            return Err(
+                vm.new_type_error("multiple bases have instance layout conflict".to_string())
+            );
+        }
     }
 
-    // FIXME: Ok(base.unwrap()) is expected
-    Ok(bases[0].clone())
+    debug_assert!(base.is_some());
+    Ok(base.unwrap())
 }
 
 #[cfg(test)]
