Trash temporary locals after destructive assignment to pass test_callbacks_on_callback, more systematic fix will be GR-64479

Author: steve-s

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java

@@ -3146,17 +3146,20 @@ public Void visit(ExprTy.Subscript node) {
 
             /**
              * This method unpacks the rhs (a sequence/iterable) to the elements on the lhs
-             * (specified by {@code nodes}.
+             * specified by {@code nodes}.
              */
             private void visitIterableAssign(ExprTy[] nodes) {
                 b.beginBlock();
 
-                /**
+                /*
                  * The rhs should be fully evaluated and unpacked into the expected number of
                  * elements before storing values into the lhs (e.g., if an lhs element is f().attr,
                  * but computing or unpacking rhs throws, f() is not computed). Thus, the unpacking
                  * step stores the unpacked values into intermediate variables, and then those
                  * variables are copied into the lhs elements afterward.
+                 *
+                 * On top of that, in order to pass the target BytecodeLocal variables as
+                 * LocalRangeAccessor, they must have consecutive indices.
                  */
                 BytecodeLocal[] targets = new BytecodeLocal[nodes.length];
                 for (int i = 0; i < targets.length; i++) {
@@ -3196,6 +3199,7 @@ private void visitIterableAssign(ExprTy[] nodes) {
                     target.accept(new StoreVisitor(() -> {
                         b.emitLoadLocal(targets[index]);
                     }));
+                    b.emitClearLocal(targets[index]);
                 }
 
                 b.endBlock();

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java

@@ -1956,6 +1956,10 @@ public static void doIt(VirtualFrame frame, Object value, Object primary, Object
         }
     }
 
+    /**
+     * This operation is used to implement destructing assignment where the rhs should be fully
+     * evaluated and unpacked into temporary variables and then assigned to the targets.
+     */
     @Operation(storeBytecodeIndex = true)
     @ConstantOperand(type = LocalRangeAccessor.class)
     @ImportStatic({PGuards.class})
@@ -1974,7 +1978,7 @@ public static void doUnpackSequence(VirtualFrame localFrame, LocalRangeAccessor
             CompilerAsserts.partialEvaluationConstant(count);
 
             if (len != count) {
-                raiseError(inliningTarget, raiseNode, len, count);
+                throw raiseError(inliningTarget, raiseNode, len, count);
             }
 
             for (int i = 0; i < count; i++) {
@@ -1983,16 +1987,15 @@ public static void doUnpackSequence(VirtualFrame localFrame, LocalRangeAccessor
         }
 
         @InliningCutoff
-        private static void raiseError(Node inliningTarget, PRaiseNode raiseNode, int len, int count) {
+        private static PException raiseError(Node inliningTarget, PRaiseNode raiseNode, int len, int count) {
             if (len < count) {
-                throw raiseNode.raise(inliningTarget, ValueError, ErrorMessages.NOT_ENOUGH_VALUES_TO_UNPACK, count, len);
+                throw raiseNotEnoughValues(inliningTarget, raiseNode, count, len);
             } else {
-                throw raiseNode.raise(inliningTarget, ValueError, ErrorMessages.TOO_MANY_VALUES_TO_UNPACK, count);
+                throw raiseTooManyValues(inliningTarget, raiseNode, count);
             }
         }
 
         @Specialization
-        @ExplodeLoop
         @InliningCutoff
         public static void doUnpackIterable(VirtualFrame virtualFrame, LocalRangeAccessor results, Object collection,
                         @Bind Node inliningTarget,
@@ -2003,29 +2006,50 @@ public static void doUnpackIterable(VirtualFrame virtualFrame, LocalRangeAccesso
                         @Exclusive @Cached PRaiseNode raiseNode) {
             int count = results.getLength();
             CompilerAsserts.partialEvaluationConstant(count);
-
             Object iterator;
             try {
                 iterator = getIter.execute(virtualFrame, inliningTarget, collection);
             } catch (PException e) {
                 e.expectTypeError(inliningTarget, notIterableProfile);
-                throw raiseNode.raise(inliningTarget, PythonBuiltinClassType.TypeError, ErrorMessages.CANNOT_UNPACK_NON_ITERABLE, collection);
+                throw raiseNotIterableError(collection, inliningTarget, raiseNode);
+            }
+            extractItems(virtualFrame, inliningTarget, bytecode, getNextNode, raiseNode, iterator, results, count);
+            try {
+                getNextNode.execute(virtualFrame, inliningTarget, iterator);
+            } catch (IteratorExhausted e) {
+                return;
             }
+            throw raiseTooManyValues(inliningTarget, raiseNode, count);
+        }
+
+        @ExplodeLoop
+        private static void extractItems(VirtualFrame virtualFrame, Node inliningTarget, BytecodeNode bytecode, PyIterNextNode getNextNode, PRaiseNode raiseNode, Object iterator,
+                        LocalRangeAccessor results, int count) {
+            CompilerAsserts.partialEvaluationConstant(count);
             for (int i = 0; i < count; i++) {
                 try {
                     Object value = getNextNode.execute(virtualFrame, inliningTarget, iterator);
                     results.setObject(bytecode, virtualFrame, i, value);
                 } catch (IteratorExhausted e) {
-                    throw raiseNode.raise(inliningTarget, ValueError, ErrorMessages.NOT_ENOUGH_VALUES_TO_UNPACK, count, i);
+                    raiseNotEnoughValues(inliningTarget, raiseNode, count, i);
                 }
             }
-            try {
-                Object value = getNextNode.execute(virtualFrame, inliningTarget, iterator);
-            } catch (IteratorExhausted e) {
-                return;
-            }
+        }
+
+        @InliningCutoff
+        private static PException raiseNotIterableError(Object collection, Node inliningTarget, PRaiseNode raiseNode) {
+            throw raiseNode.raise(inliningTarget, PythonBuiltinClassType.TypeError, ErrorMessages.CANNOT_UNPACK_NON_ITERABLE, collection);
+        }
+
+        @InliningCutoff
+        private static PException raiseTooManyValues(Node inliningTarget, PRaiseNode raiseNode, int count) {
             throw raiseNode.raise(inliningTarget, ValueError, ErrorMessages.TOO_MANY_VALUES_TO_UNPACK, count);
         }
+
+        @InliningCutoff
+        private static PException raiseNotEnoughValues(Node inliningTarget, PRaiseNode raiseNode, int expected, int actual) {
+            throw raiseNode.raise(inliningTarget, ValueError, ErrorMessages.NOT_ENOUGH_VALUES_TO_UNPACK, expected, actual);
+        }
     }
 
     @Operation(storeBytecodeIndex = true)