Detect invalid returns in module bodies

* Such as `class C; proc { return 42 }.call; end`.

Author: eregon

spec/tags/language/return_tags.txt

@@ -3,10 +3,7 @@ slow:The return keyword at top level stops file execution
 slow:The return keyword at top level within a begin fires ensure block before returning
 slow:The return keyword at top level return with argument does not affect exit status
 fails:The return keyword at top level within a class is allowed
-fails:The return keyword at top level file loading stops file loading and execution
-fails:The return keyword at top level file requiring stops file loading and execution
 fails:The return keyword at top level return with argument does not affect exit status
 fails:The return keyword at top level within a block within a class is allowed
-fails(escapes the interpreter):The return keyword at top level within a block within a class is not allowed
 slow:The return keyword at top level return with argument warns but does not affect exit status
 fails:The return keyword at top level return with argument warns but does not affect exit status

src/main/java/org/truffleruby/language/control/InvalidReturnNode.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 2.0, or
+ * GNU General Public License version 2, or
+ * GNU Lesser General Public License version 2.1.
+ */
+package org.truffleruby.language.control;
+
+import org.truffleruby.language.RubyContextSourceNode;
+import org.truffleruby.language.RubyNode;
+
+import com.oracle.truffle.api.frame.VirtualFrame;
+
+public class InvalidReturnNode extends RubyContextSourceNode {
+
+    @Child public RubyNode value;
+
+    public InvalidReturnNode(RubyNode value) {
+        this.value = value;
+    }
+
+    @Override
+    public Object execute(VirtualFrame frame) {
+        value.doExecuteVoid(frame);
+        throw new RaiseException(getContext(), coreExceptions().unexpectedReturn(this));
+    }
+
+}

src/main/java/org/truffleruby/language/control/ReturnID.java

@@ -9,6 +9,10 @@
  */
 package org.truffleruby.language.control;
 
-public class ReturnID {
+public final class ReturnID {
+
+    /** Returning to or through class/module bodies is a LocalJumpError. However, the surrounding block might be turned
+     * into a lambda, in which case it becomes valid and returns from the lambda. */
+    public static final ReturnID MODULE_BODY = new ReturnID();
 
 }

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -85,6 +85,7 @@
 import org.truffleruby.language.control.FrameOnStackNode;
 import org.truffleruby.language.control.IfElseNode;
 import org.truffleruby.language.control.IfNode;
+import org.truffleruby.language.control.InvalidReturnNode;
 import org.truffleruby.language.control.LocalReturnNode;
 import org.truffleruby.language.control.NextNode;
 import org.truffleruby.language.control.NotNode;
@@ -846,18 +847,10 @@ private RubyNode openModule(SourceIndexLength sourceSection, RubyNode defineOrGe
                     null,
                     null);
 
-            final ReturnID returnId;
-
-            if (type == OpenModule.SINGLETON_CLASS) {
-                returnId = environment.getReturnID();
-            } else {
-                returnId = environment.getParseEnvironment().allocateReturnID();
-            }
-
             final TranslatorEnvironment newEnvironment = new TranslatorEnvironment(
                     environment,
                     environment.getParseEnvironment(),
-                    returnId,
+                    ReturnID.MODULE_BODY,
                     true,
                     true,
                     true,
@@ -2872,7 +2865,12 @@ public RubyNode visitReturnNode(ReturnParseNode node) {
         final RubyNode ret;
 
         if (environment.isBlock()) {
-            ret = new DynamicReturnNode(environment.getReturnID(), translatedChild);
+            final ReturnID returnID = environment.getReturnID();
+            if (returnID == ReturnID.MODULE_BODY) {
+                ret = new InvalidReturnNode(translatedChild);
+            } else {
+                ret = new DynamicReturnNode(returnID, translatedChild);
+            }
         } else {
             ret = new LocalReturnNode(translatedChild);
         }

src/main/java/org/truffleruby/parser/MethodTranslator.java

@@ -24,7 +24,9 @@
 import org.truffleruby.language.arguments.ReadPreArgumentNode;
 import org.truffleruby.language.arguments.ShouldDestructureNode;
 import org.truffleruby.language.control.AndNode;
+import org.truffleruby.language.control.DynamicReturnNode;
 import org.truffleruby.language.control.IfElseNode;
+import org.truffleruby.language.control.InvalidReturnNode;
 import org.truffleruby.language.control.NotNode;
 import org.truffleruby.language.control.SequenceNode;
 import org.truffleruby.language.locals.FlipFlopStateNode;
@@ -172,9 +174,9 @@ public BlockDefinitionNode compileBlockNode(SourceIndexLength sourceSection, Par
         body = new ExceptionTranslatingNode(body, UnsupportedOperationBehavior.TYPE_ERROR);
 
         // Procs
-        final RubyNode bodyProc = new CatchForProcNode(
-                composeBody(sourceSection, preludeProc, NodeUtil.cloneNode(body)));
-        bodyProc.unsafeSetSourceSection(enclosing(sourceSection, body));
+        final RubyNode bodyCopyForProc = NodeUtil.cloneNode(body);
+        final RubyNode bodyProc = new CatchForProcNode(composeBody(sourceSection, preludeProc, bodyCopyForProc));
+        bodyProc.unsafeSetSourceSection(enclosing(sourceSection, bodyCopyForProc));
 
         final RubyRootNode newRootNodeForProcs = new RubyRootNode(
                 language,
@@ -185,7 +187,8 @@ public BlockDefinitionNode compileBlockNode(SourceIndexLength sourceSection, Par
                 Split.HEURISTIC);
 
         // Lambdas
-        RubyNode composed = composeBody(sourceSection, preludeLambda, body /* no copy, last usage */);
+        RubyNode bodyForLambda = body; // no copy, last usage
+        RubyNode composed = composeBody(sourceSection, preludeLambda, bodyForLambda);
 
         composed = new CatchForLambdaNode(environment.getReturnID(), environment.getBreakID(), composed);
 
@@ -201,6 +204,14 @@ public BlockDefinitionNode compileBlockNode(SourceIndexLength sourceSection, Par
         final RootCallTarget callTargetAsLambda = Truffle.getRuntime().createCallTarget(newRootNodeForLambdas);
         final RootCallTarget callTargetAsProc = Truffle.getRuntime().createCallTarget(newRootNodeForProcs);
 
+        if (isProc) {
+            // If we end up executing this block as a lambda, but don't know it statically, e.g., `lambda {}` or
+            // `define_method(:foo, proc {})`), then returns are always valid and return from that lambda.
+            // This needs to run after nodes are adopted for replace() to work and nodes to know their parent.
+            for (InvalidReturnNode returnNode : NodeUtil.findAllNodeInstances(bodyForLambda, InvalidReturnNode.class)) {
+                returnNode.replace(new DynamicReturnNode(environment.getReturnID(), returnNode.value));
+            }
+        }
 
         Object frameOnStackMarkerSlot;