Cleanup code

Author: Adam Hrbac

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/FrameBuiltins.java

@@ -43,6 +43,8 @@
 import com.oracle.graal.python.builtins.objects.object.ObjectBuiltinsFactory;
 import com.oracle.graal.python.builtins.objects.object.PythonObject;
 import com.oracle.graal.python.builtins.objects.str.StringUtils.SimpleTruffleStringFormatNode;
+import com.oracle.graal.python.lib.PyLongAsLongAndOverflowNode;
+import com.oracle.graal.python.lib.PyLongCheckExactNode;
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.frame.GetFrameLocalsNode;
@@ -55,6 +57,7 @@
 import com.oracle.graal.python.nodes.util.CannotCastException;
 import com.oracle.graal.python.nodes.util.CastToJavaBooleanNode;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
+import com.oracle.graal.python.util.OverflowException;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.RootCallTarget;
 import com.oracle.truffle.api.dsl.Bind;
@@ -142,6 +145,7 @@ public static GetBuiltinsNode create() {
 
     @GenerateNodeFactory
     public abstract static class GetLinenoNode extends PythonBuiltinNode {
+        // Kept around since it is used by other nodes
         public abstract int executeInt(VirtualFrame frame, PFrame self);
 
         @Specialization
@@ -168,14 +172,19 @@ public static GetLinenoNode create() {
 
     @Builtin(name = "f_lineno", minNumOfPositionalArgs = 1, maxNumOfPositionalArgs = 2, isGetter = true, isSetter = true)
     @GenerateNodeFactory
-    public abstract static class SetLinenoNode extends PythonBuiltinNode {
+    public abstract static class LinenoNode extends PythonBuiltinNode {
         public abstract Object execute(VirtualFrame frame, PFrame self, Object newLineno);
 
         @Specialization(guards = "isNoValue(newLineno)")
         int get(VirtualFrame frame, PFrame self, Object newLineno,
                         @Bind("this") Node inliningTarget,
                         @Cached.Shared("isCurrentFrame") @Cached InlinedConditionProfile isCurrentFrameProfile,
                         @Cached.Shared("materialize") @Cached MaterializeFrameNode materializeNode) {
+            syncLocationIfNeeded(frame, self, inliningTarget, isCurrentFrameProfile, materializeNode);
+            return self.getLine();
+        }
+
+        private void syncLocationIfNeeded(VirtualFrame frame, PFrame self, Node inliningTarget, InlinedConditionProfile isCurrentFrameProfile, MaterializeFrameNode materializeNode) {
             // Special case because this builtin can be called without going through an invoke node:
             // we need to sync the location of the frame if and only if 'self' represents the
             // current frame. If 'self' represents another frame on the stack, the location is
@@ -184,28 +193,31 @@ int get(VirtualFrame frame, PFrame self, Object newLineno,
                 PFrame pyFrame = materializeNode.execute(frame, this, false, false);
                 assert pyFrame == self;
             }
-            return self.getLine();
         }
 
         @Specialization(guards = "!isNoValue(newLineno)")
         PNone set(VirtualFrame frame, PFrame self, Object newLineno,
                         @Bind("this") Node inliningTarget,
                         @Cached.Shared("isCurrentFrame") @Cached InlinedConditionProfile isCurrentFrameProfile,
                         @Cached.Shared("materialize") @Cached MaterializeFrameNode materializeNode,
-                        @Cached PRaiseNode.Lazy raise) {
-            // Special case because this builtin can be called without going through an invoke node:
-            // we need to sync the location of the frame if and only if 'self' represents the
-            // current frame. If 'self' represents another frame on the stack, the location is
-            // already set
-            if (isCurrentFrameProfile.profile(inliningTarget, frame != null && PArguments.getCurrentFrameInfo(frame) == self.getRef())) {
-                PFrame pyFrame = materializeNode.execute(frame, this, false, false);
-                assert pyFrame == self;
-            }
+                        @Cached PRaiseNode.Lazy raise,
+                        @Cached PyLongCheckExactNode isLong,
+                  @Cached PyLongAsLongAndOverflowNode toLong) {
+            syncLocationIfNeeded(frame, self, inliningTarget, isCurrentFrameProfile, materializeNode);
             if (self.isTraceArgument()) {
-                if (newLineno instanceof Integer x) {
-                    self.setJumpDestLine(x);
+                if (isLong.execute(inliningTarget, newLineno)) {
+                    try {
+                        long lineno = toLong.execute(frame, inliningTarget, newLineno);
+                        if (lineno <= Integer.MAX_VALUE && lineno >= Integer.MIN_VALUE) {
+                            self.setJumpDestLine((int)lineno);
+                        } else {
+                            throw raise.get(inliningTarget).raise(PythonBuiltinClassType.ValueError, ErrorMessages.LINENO_OUT_OF_RANGE);
+                        }
+                    } catch (OverflowException e) {
+                        throw raise.get(inliningTarget).raise(PythonBuiltinClassType.ValueError, ErrorMessages.LINENO_OUT_OF_RANGE);
+                    }
                 } else {
-                    throw raise.get(inliningTarget).raise(PythonBuiltinClassType.ValueError, ErrorMessages.EXPECTED_S_GOT_P, "int", newLineno);
+                    throw raise.get(inliningTarget).raise(PythonBuiltinClassType.ValueError, ErrorMessages.LINENO_MUST_BE_AN_INTEGER)
                 }
             } else {
                 throw raise.get(inliningTarget).raise(PythonBuiltinClassType.ValueError, ErrorMessages.CANT_JUMP_FROM_S_EVENT, getContext().getThreadState(getLanguage()).getTracingWhat().pythonName);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/CodeUnit.java

@@ -629,7 +629,7 @@ public List<ArrayList<StackItem>> computeStackElems() {
         todo.addFirst(0);
         while (!todo.isEmpty()) {
             int i = todo.removeLast();
-            assert blocks.get(i) != null : "TODO message here";
+            assert blocks.get(i) != null : "Reached block without determining its stack state";
             opCodeAt(code, i, (bci, op, oparg, followingArgs) -> {
                 ArrayList<StackItem> next = blocks.get(bci);
                 for (int j = 0; j < exceptionHandlerRanges.length; j += 4) {
@@ -692,8 +692,19 @@ public List<ArrayList<StackItem>> computeStackElems() {
                     }
                     case LOAD_NONE:
                         opCodeAt(code, op.getNextBci(bci, oparg, false), (ignored, nextOp, ignored2, ignored3) -> {
-                            // used instead of exceptions in non-error paths for these opcodes
-                            // the handler code will push the Except StackItem
+                            // Usually, when compiling bytecode around exception handlers, the code
+                            // is generated twice, once for the path with no exception, and
+                            // once for the path with the exception. However, when generating code
+                            // for a with statement exit, the code is generated as follows (and in a
+                            // similar manner for async with).
+                            // ...
+                            // LOAD_NONE
+                            // EXIT_WITH (exception handler starts here)
+                            // ...
+                            // This means that setting the stack at EXIT_WITH to have Object on top,
+                            // as LOAD_NONE usually would, would cause a conflict with the exception
+                            // handler starting at that position, which has the stack top be an
+                            // Exception.
                             if (nextOp != OpCodes.GET_AEXIT_CORO && nextOp != OpCodes.EXIT_WITH) {
                                 setNextStack(todo, blocks, op.getNextBci(bci, oparg, false), StackItem.Object.push(blocks.get(bci)));
                             }
@@ -722,16 +733,12 @@ public List<ArrayList<StackItem>> computeStackElems() {
     private void handleGeneralOp(List<ArrayList<StackItem>> blocks, ArrayDeque<Integer> todo, int bci, int next, int stackLost, int stackGain) {
         if (next >= 0) {
             ArrayList<StackItem> blocksHere = new ArrayList<>(blocks.get(bci));
-            // System.out.println("Before: " + stackLost + "\t" + stackGain + "\t" + bci + "\t" +
-            // blocksHere);
             for (int k = 0; k < stackLost; ++k) {
                 blocksHere.remove(blocksHere.size() - 1);
             }
             for (int k = 0; k < stackGain; ++k) {
                 blocksHere.add(StackItem.Object);
             }
-            // System.out.println("Before stack: " + blocks.get(next));
-            // System.out.println("After: " + blocksHere);
             setNextStack(todo, blocks, next, blocksHere);
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

@@ -1620,5 +1620,7 @@ public abstract class ErrorMessages {
     public static final TruffleString LINE_D_COMES_AFTER_THE_CURRENT_CODE_BLOCK = tsLiteral("line %d comes after the current code block");
     public static final TruffleString LINE_D_COMES_BEFORE_THE_CURRENT_CODE_BLOCK = tsLiteral("line %d comes before the current code block");
     public static final TruffleString CANT_JUMP_FROM_S_EVENT = tsLiteral("Can't jump from \"%s\" event.");
+    public static final TruffleString LINENO_MUST_BE_AN_INTEGER = tsLiteral("lineno must be an integer");
 
+    public static final TruffleString LINENO_OUT_OF_RANGE = tsLiteral("lineno out of range");
 }