Fix inconsistencies in frame materialization

Author: msimacek

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

@@ -250,9 +250,14 @@ static HashingStorage cached(LocalsStorage self, HashingStorage other,
                         @Exclusive @SuppressWarnings("unused") @Cached("self.frame.getFrameDescriptor()") FrameDescriptor desc) {
             HashingStorage result = other;
             for (int slot = 0; slot < desc.getNumberOfSlots(); slot++) {
-                Object value = self.getValue(slot);
-                if (value != null) {
-                    result = lib.setItem(result, desc.getSlotName(slot), value);
+                Object identifier = desc.getSlotName(slot);
+                if (identifier instanceof String) {
+                    if (isUserFrameSlot(identifier)) {
+                        Object value = self.getValue(slot);
+                        if (value != null) {
+                            result = lib.setItem(result, desc.getSlotName(slot), value);
+                        }
+                    }
                 }
             }
             return result;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java

@@ -1872,10 +1872,10 @@ private int bytecodeRaiseVarargs(VirtualFrame virtualFrame, Frame stackFrame, in
     private void raiseUnboundCell(Node[] localNodes, int bci, int oparg) {
         PRaiseNode raiseNode = insertChildNode(localNodes, bci, UNCACHED_RAISE, NODE_RAISE);
         if (oparg < freeoffset) {
-            int varIdx = oparg - celloffset;
+            int varIdx = oparg;
             throw raiseNode.raise(PythonBuiltinClassType.UnboundLocalError, ErrorMessages.LOCAL_VAR_REFERENCED_BEFORE_ASSIGMENT, cellvars[varIdx]);
         } else {
-            int varIdx = oparg - freeoffset;
+            int varIdx = oparg - cellvars.length;
             throw raiseNode.raise(PythonBuiltinClassType.NameError, ErrorMessages.UNBOUNDFREEVAR, freevars[varIdx]);
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java

@@ -40,7 +40,6 @@
  */
 package com.oracle.graal.python.nodes.frame;
 
-import com.oracle.graal.python.builtins.PythonBuiltinClassType;
 import com.oracle.graal.python.builtins.objects.cell.PCell;
 import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
 import com.oracle.graal.python.builtins.objects.common.HashingStorage;
@@ -50,12 +49,12 @@
 import com.oracle.graal.python.builtins.objects.frame.PFrame;
 import com.oracle.graal.python.builtins.objects.function.PArguments;
 import com.oracle.graal.python.nodes.ModuleRootNode;
+import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PRootNode;
 import com.oracle.graal.python.nodes.SpecialMethodNames;
 import com.oracle.graal.python.nodes.bytecode.FrameInfo;
 import com.oracle.graal.python.nodes.frame.MaterializeFrameNodeGen.SyncFrameValuesNodeGen;
 import com.oracle.graal.python.nodes.function.ClassBodyRootNode;
-import com.oracle.graal.python.nodes.object.GetClassNode;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.CompilerDirectives;
@@ -432,13 +431,6 @@ static void doGenericDictAST(VirtualFrame frame, PFrame pyFrame, Frame frameToSy
             }
         }
 
-        @Specialization(guards = {"isCustomLocalsObject(pyFrame, frameToSync, frameProfile)"}, limit = "1")
-        @SuppressWarnings("unused")
-        static void doCustomLocalsObject(PFrame pyFrame, Frame frameToSync, @SuppressWarnings("unused") Node location,
-                        @Cached("createClassProfile()") ValueProfile frameProfile) {
-            // nothing to do; we already worked on the custom object
-        }
-
         @Specialization(guards = {"isBytecodeFrame(frameToSync)", "hasLocalsStorage(pyFrame, frameToSync, frameProfile)", "frameToSync.getFrameDescriptor() == cachedFd",
                         "variableSlotCount(cachedFd) < 32"}, limit = "1")
         @ExplodeLoop
@@ -550,6 +542,12 @@ static void doGenericDict(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
             }
         }
 
+        @Specialization(guards = "!isDictWithCustomStorage(pyFrame)")
+        @SuppressWarnings("unused")
+        static void doCustomLocalsObject(PFrame pyFrame, Frame frameToSync, Node location) {
+            // nothing to do; we already worked on the custom object
+        }
+
         private static void syncDict(VirtualFrame frame, int slot, FrameInfo info, Frame frameToSync, PDict localsDict, HashingStorageLibrary lib,
                         ConditionProfile hasFrame, BranchProfile updatedStorage, ConditionProfile profile) {
             HashingStorage storage = localsDict.getDictStorage();
@@ -558,21 +556,25 @@ private static void syncDict(VirtualFrame frame, int slot, FrameInfo info, Frame
             if (value != null && profile.profile(value instanceof PCell)) {
                 value = ((PCell) value).getRef();
             }
-            HashingStorage newStore = null;
+            HashingStorage newStore;
             if (value != null) {
                 newStore = lib.setItemWithFrame(storage, identifier, value, hasFrame, frame);
+                if (newStore != storage) {
+                    updatedStorage.enter();
+                    localsDict.setDictStorage(newStore);
+                }
             } else {
                 // delete variable
                 // TODO: FIXME: this might call __hash__ twice
                 boolean hasKey = lib.hasKeyWithFrame(storage, identifier, hasFrame, frame);
                 if (hasKey) {
                     newStore = lib.delItemWithFrame(storage, identifier, hasFrame, frame);
+                    if (newStore != storage) {
+                        updatedStorage.enter();
+                        localsDict.setDictStorage(newStore);
+                    }
                 }
             }
-            if (newStore != storage) {
-                updatedStorage.enter();
-                localsDict.setDictStorage(newStore);
-            }
         }
 
         // @ImportStatic doesn't work on this for some reason
@@ -606,11 +608,7 @@ protected static boolean hasLocalsStorage(PFrame pyFrame, Frame frameToSync, Val
         protected static boolean isDictWithCustomStorage(PFrame pyFrame) {
             Object localsObject = pyFrame.getLocalsDict();
             // do not allow subclasses of 'dict'
-            return GetClassNode.getUncached().execute(localsObject) == PythonBuiltinClassType.PDict;
-        }
-
-        protected static boolean isCustomLocalsObject(PFrame pyFrame, Frame frameToSync, ValueProfile frameProfile) {
-            return !hasLocalsStorage(pyFrame, frameToSync, frameProfile);
+            return localsObject instanceof PDict && PGuards.isBuiltinDict((PDict) localsObject);
         }
 
         protected static LocalsStorage getLocalsStorage(PFrame pyFrame) {