[GR-36883] Fix multiple bases with instance lay-out conflict.

PullRequest: graalpython/2143

Author: cosminbasca

graalpython/com.oracle.graal.python.test/src/tests/test_type.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -153,6 +153,7 @@ class C(A, B): pass
 #    assert raised
 #    assert C.__bases__ == [object]
 
+
 def test_namespace_with_non_string_keys():
     class MyStr(str):
         pass
@@ -162,6 +163,7 @@ class MyStr(str):
     })
     assert any(type(k) == MyStr for k in A.__dict__.keys())
 
+
 def test_mro():
     class M(type):
         def mro(cls):
@@ -173,7 +175,8 @@ class B: pass
     class C(A, B, metaclass = M): pass
 
     assert C.__mro__ == (C, B, A, object)
-    
+
+
 def test_dir_sorted():
     class C:
         b = 1
@@ -253,6 +256,7 @@ class MyDict(dict):
     ]:
         assert type(x).__flags__ & flag, "masked __flags__ = {}, expected {}".format(type(x).__flags__ & flag, flag)
 
+
 def test_dict():
     def dict_element_raises(o, err):
         raised = False
@@ -295,7 +299,8 @@ class SubSlots(BaseSlots, Base):
 
     str(type(SubSlots.__dict__['__dict__'])) == "<class 'get_set_desc'>"
     assert SubSlots().__dict__ == {}
-        
+
+
 def test_itemsize():
     assert object.__itemsize__ == 0
     assert list.__itemsize__ == 0
@@ -330,7 +335,8 @@ class C():
     class C(tuple):
         __itemsize__ = 42
     assert C.__itemsize__ == 8
-    
+
+
 def test_descr_name_qualname():    
     assert float.real.__qualname__ == 'float.real'
     assert float.real.__name__ == 'real'
@@ -349,7 +355,8 @@ class C: __slots__ = ['a']
     except AttributeError:
         raised = True
     assert raised    
-    
+
+
 def test_cant_set_builtin_attributes():
     raised = False
     try:
@@ -392,3 +399,41 @@ def test_cant_set_builtin_attributes():
     except TypeError:
         raised = True
     assert raised    
+
+
+def test_slots_no_instance_layout_conflict():
+    # with slots
+    class A(object):
+        __slots__ = ("a", "b")
+
+    class B(A):
+        pass
+
+    class C(A):
+        pass
+
+    raised = False
+    try:
+        class D(B, C):
+            pass
+    except TypeError:
+        raised = True
+    assert not raised
+
+    # without slots
+    class A(object):
+        pass
+
+    class B(A):
+        pass
+
+    class C(A):
+        pass
+
+    raised = False
+    try:
+        class D(B, C):
+            pass
+    except TypeError:
+        raised = True
+    assert not raised

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021, Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates.
  * Copyright (c) 2013, Regents of the University of California
  *
  * All rights reserved.
@@ -3624,6 +3624,10 @@ private static Object[] materializeGeneric(SequenceStorage s, int len, GetItemSc
             }
             return barr;
         }
+
+        public static GetInternalObjectArrayNode getUncached() {
+            return SequenceStorageNodesFactory.GetInternalObjectArrayNodeGen.getUncached();
+        }
     }
 
     @GenerateUncached

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java

@@ -1104,30 +1104,29 @@ static GetSolidBaseNode getUncached() {
         protected Object getSolid(Object type,
                         @Cached GetBaseClassNode getBaseClassNode,
                         @Cached("createForceType()") ReadAttributeFromObjectNode readAttr,
-                        @Cached GetInternalObjectArrayNode getArrayNode,
                         @Cached BranchProfile typeIsNotBase,
                         @Cached BranchProfile hasBase,
                         @Cached BranchProfile hasNoBase) {
-            return solidBase(type, getBaseClassNode, PythonContext.get(this), readAttr, getArrayNode, typeIsNotBase, hasBase,
+            return solidBase(type, getBaseClassNode, PythonContext.get(this), readAttr, typeIsNotBase, hasBase,
                             hasNoBase, 0);
         }
 
         @TruffleBoundary
-        protected Object solidBaseTB(Object type, GetBaseClassNode getBaseClassNode, PythonContext context, GetInternalObjectArrayNode getArrayNode, int depth) {
-            return solidBase(type, getBaseClassNode, context, ReadAttributeFromObjectNode.getUncachedForceType(), getArrayNode, BranchProfile.getUncached(),
+        protected Object solidBaseTB(Object type, GetBaseClassNode getBaseClassNode, PythonContext context, int depth) {
+            return solidBase(type, getBaseClassNode, context, ReadAttributeFromObjectNode.getUncachedForceType(), BranchProfile.getUncached(),
                             BranchProfile.getUncached(), BranchProfile.getUncached(), depth);
         }
 
         protected Object solidBase(Object type, GetBaseClassNode getBaseClassNode, PythonContext context, ReadAttributeFromObjectNode readAttr,
-                        GetInternalObjectArrayNode getArrayNode, BranchProfile typeIsNotBase, BranchProfile hasBase, BranchProfile hasNoBase, int depth) {
+                        BranchProfile typeIsNotBase, BranchProfile hasBase, BranchProfile hasNoBase, int depth) {
             CompilerAsserts.partialEvaluationConstant(depth);
             Object base = getBaseClassNode.execute(type);
             if (base != null) {
                 hasBase.enter();
                 if (depth > 3) {
-                    base = solidBaseTB(base, getBaseClassNode, context, getArrayNode, depth);
+                    base = solidBaseTB(base, getBaseClassNode, context, depth);
                 } else {
-                    base = solidBase(base, getBaseClassNode, context, readAttr, getArrayNode, typeIsNotBase, hasBase,
+                    base = solidBase(base, getBaseClassNode, context, readAttr, typeIsNotBase, hasBase,
                                     hasNoBase, depth + 1);
                 }
             } else {
@@ -1141,18 +1140,16 @@ protected Object solidBase(Object type, GetBaseClassNode getBaseClassNode, Pytho
             typeIsNotBase.enter();
 
             Object typeSlots = getSlotsFromType(type, readAttr);
-            Object baseSlots = getSlotsFromType(base, readAttr);
-            if (extraivars(type, base, typeSlots, baseSlots, getArrayNode)) {
+            if (extraivars(type, base, typeSlots)) {
                 return type;
             } else {
                 return base;
             }
         }
 
         @TruffleBoundary
-        private static boolean extraivars(Object type, Object base, Object typeSlots, Object baseSlots, GetInternalObjectArrayNode getArrayNode) {
-            if (typeSlots == null && baseSlots != null && length(((PSequence) baseSlots).getSequenceStorage(), getArrayNode) != 0 ||
-                            baseSlots == null && typeSlots != null && length(((PSequence) typeSlots).getSequenceStorage(), getArrayNode) != 0) {
+        private static boolean extraivars(Object type, Object base, Object typeSlots) {
+            if (typeSlots != null && length(typeSlots) != 0) {
                 return true;
             }
             Object typeNewMethod = LookupAttributeInMRONode.lookup(type, __NEW__, GetMroStorageNode.getUncached(), ReadAttributeFromObjectNode.getUncached(), true);
@@ -1161,18 +1158,26 @@ private static boolean extraivars(Object type, Object base, Object typeSlots, Ob
         }
 
         @TruffleBoundary
-        private static int length(SequenceStorage storage, GetInternalObjectArrayNode getArrayNode) {
-            int result = 0;
-            int length = storage.length();
-            Object[] slots = getArrayNode.execute(storage);
-            for (int i = 0; i < length; i++) {
-                // omit __DICT__ and __WEAKREF__, they cause no class layout conflict
-                // see also test_slts.py#test_no_bases_have_class_layout_conflict
-                if (!(slots[i].equals(__DICT__) || slots[i].equals(__WEAKREF__))) {
-                    result++;
+        private static int length(Object slotsObject) {
+            assert PGuards.isString(slotsObject) || PGuards.isPSequence(slotsObject) : "slotsObject must be either a String or a PSequence";
+
+            if (PGuards.isString(slotsObject)) {
+                return (slotsObject.equals(__DICT__) || slotsObject.equals(__WEAKREF__)) ? 0 : 1;
+            } else {
+                SequenceStorage storage = ((PSequence) slotsObject).getSequenceStorage();
+
+                int count = 0;
+                int length = storage.length();
+                Object[] slots = GetInternalObjectArrayNode.getUncached().execute(storage);
+                for (int i = 0; i < length; i++) {
+                    // omit __DICT__ and __WEAKREF__, they cause no class layout conflict
+                    // see also test_slts.py#test_no_bases_have_class_layout_conflict
+                    if (!(slots[i].equals(__DICT__) || slots[i].equals(__WEAKREF__))) {
+                        count++;
+                    }
                 }
+                return count;
             }
-            return result;
         }
 
         private static Object getSlotsFromType(Object type, ReadAttributeFromObjectNode readAttr) {