Bytecode DSL fix: last expr statement in module is return value for interop eval even if it is a docstring

Author: steve-s

graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/basic/HelloWorldTests.java

@@ -32,6 +32,8 @@
 
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Engine;
+import org.graalvm.polyglot.Value;
+import org.junit.Assert;
 import org.junit.Test;
 
 import com.oracle.graal.python.test.integration.PythonTests;
@@ -50,6 +52,12 @@ public void helloworldAgain() {
         PythonTests.assertPrints("hello\n", source);
     }
 
+    @Test
+    public void helloworldExpression() {
+        Value value = PythonTests.eval("'hello world'");
+        Assert.assertEquals("hello world", value.asString());
+    }
+
     @Test
     public void usesFrozenModules() {
         final ByteArrayOutputStream byteArray = new ByteArrayOutputStream();

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

@@ -772,7 +772,7 @@ String maybeMangleAndAddName(String name) {
     public BytecodeDSLCompilerResult visit(ModTy.Module node) {
         return compileRootNode("<module>", ArgumentInfo.NO_ARGS, node.getSourceRange(), b -> {
             beginRootNode(node, null, b);
-            visitModuleBody(node.body, b);
+            visitModuleBody(node.body, b, true);
             endRootNode(b);
         });
     }
@@ -792,12 +792,12 @@ public BytecodeDSLCompilerResult visit(ModTy.Expression node) {
     public BytecodeDSLCompilerResult visit(ModTy.Interactive node) {
         return compileRootNode("<module>", ArgumentInfo.NO_ARGS, node.getSourceRange(), b -> {
             beginRootNode(node, null, b);
-            visitModuleBody(node.body, b);
+            visitModuleBody(node.body, b, false);
             endRootNode(b);
         });
     }
 
-    private void visitModuleBody(StmtTy[] body, Builder b) {
+    private void visitModuleBody(StmtTy[] body, Builder b, boolean returnLastStmt) {
         if (body != null) {
             if (containsAnnotations(body)) {
                 b.emitSetupAnnotations();
@@ -834,33 +834,30 @@ private void visitModuleBody(StmtTy[] body, Builder b) {
                         endStoreLocal("__doc__", b);
                     }
                 }
-                if (i == body.length) {
-                    // Special case: module body just consists of a docstring.
+
+                for (; i < body.length - 1; i++) {
+                    body[i].accept(statementCompiler);
+                }
+
+                /*
+                 * To support interop eval we need to return the value of the last statement even if
+                 * we're in file mode. Also used when parsing with arguments. Note that if there is
+                 * only a doc string, although we normally skip it, here we use it as a return
+                 * value.
+                 */
+                StmtTy lastStatement = body[body.length - 1];
+                if (returnLastStmt && lastStatement instanceof StmtTy.Expr expr) {
+                    // Return the value of the last statement for interop eval.
+                    beginReturn(b);
+                    boolean closeTag = beginSourceSection(expr, b);
+                    expr.value.accept(statementCompiler);
+                    endSourceSection(b, closeTag);
+                    endReturn(b);
+                } else {
+                    lastStatement.accept(statementCompiler);
                     beginReturn(b);
                     b.emitLoadConstant(PNone.NONE);
                     endReturn(b);
-                    return;
-                }
-
-                for (; i < body.length; i++) {
-                    StmtTy bodyNode = body[i];
-                    if (i == body.length - 1) {
-                        if (bodyNode instanceof StmtTy.Expr expr) {
-                            // Return the value of the last statement for interop eval.
-                            beginReturn(b);
-                            boolean closeTag = beginSourceSection(expr, b);
-                            expr.value.accept(statementCompiler);
-                            endSourceSection(b, closeTag);
-                            endReturn(b);
-                        } else {
-                            bodyNode.accept(statementCompiler);
-                            beginReturn(b);
-                            b.emitLoadConstant(PNone.NONE);
-                            endReturn(b);
-                        }
-                    } else {
-                        bodyNode.accept(statementCompiler);
-                    }
                 }
             }
         } else {