Fix #1690: Identify define_method scopes to catch nonlocal returns

* The old check was walking past all closure scopes which meant it was walking past method bodies defined via define_method.
jruby · Sep 11, 2014 · 332cb2a · 332cb2a · subbuss · Sep 11, 2014
1 parent 9062455
commit 332cb2a
Showing 2 changed files with 22 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -56,6 +56,7 @@ public static boolean inProc(Block.Type blockType) {
     public static void initiateNonLocalReturn(ThreadContext context, DynamicScope dynScope, boolean maybeLambda, IRubyObject returnValue) {
         IRStaticScope scope = (IRStaticScope)dynScope.getStaticScope();
         IRScopeType scopeType = scope.getScopeType();
+        boolean inDefineMethod = false;
         while (dynScope != null) {
             IRStaticScope ss = (IRStaticScope)dynScope.getStaticScope();
             // SSS FIXME: Why is scopeType empty? Looks like this static-scope
@@ -65,8 +66,13 @@ public static void initiateNonLocalReturn(ThreadContext context, DynamicScope dy
             //
             // To be investigated.
             IRScopeType ssType = ss.getScopeType();
-            if (ssType != null && ssType.isMethodType()) {
-                break;
+            if (ssType != null) {
+                if (ssType.isMethodType()) {
+                    break;
+                } else if (ss.isArgumentScope() && ssType.isClosureType() && ssType != IRScopeType.EVAL_SCRIPT) {
+                    inDefineMethod = true;
+                    break;
+                }
             }
             dynScope = dynScope.getNextCapturedScope();
         }
@@ -77,7 +83,7 @@ public static void initiateNonLocalReturn(ThreadContext context, DynamicScope dy
         // Ruby code: lambda { Thread.new { return }.join }.call
         //
         // To be investigated.
-        if (   (scopeType == null || (scopeType.isClosureType() && scopeType != IRScopeType.EVAL_SCRIPT))
+        if (   (scopeType == null || (!inDefineMethod && scopeType.isClosureType() && scopeType != IRScopeType.EVAL_SCRIPT))
             && (maybeLambda || !context.scopeExistsOnCallStack(dynScope)))
         {
             // Cannot return from the call that we have long since exited.

diff --git a/spec/regression/GH-1690_nonlocal_return_in_define_method_block.rb b/spec/regression/GH-1690_nonlocal_return_in_define_method_block.rb
@@ -0,0 +1,13 @@
+# https://github.com/jruby/jruby/issues/1690
+
+def yielder; yield; end
+
+class C
+  define_method(:foo) { yielder { return :foo; } }
+end
+
+describe 'NonLocalReturn' do
+  it 'inside a block of a define_method block body returns normally' do
+    C.new.foo.should_be :foo
+  end
+end