Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 3a4eefb04d53
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 8e4ba2145ead
Choose a head ref
  • 3 commits
  • 7 files changed
  • 1 contributor

Commits on Nov 8, 2017

  1. Fix block escaping logic in interpreter one arg block calls. 

    The "full" interpreter inlines some call logic to avoid extra
dispatch. In this case, the logic it inlined did not include
logic to only escape the block if it were passed as a literal
block. Instead, it escaped it all the time. Interestingly, the
actual instruction (used by the startup interpreter) was wrong
in the opposite way: it never escaped the block.

Both places have been fixed to do the right thing and comments
added to help ensure they remain the same.

Fixes #4841.
    @headius
    headius committed Nov 8, 2017
    Copy the full SHA
    c7ce51d View commit details

  2. Add spec for looped delegation of a block containing a break. 

    In #4841 we found that JRuby was incorrectly marking
a delegated block (passed via &block) as having "escaped". We use
this escape bit to know whether non-local flow control like
break has a valid target, and so prematurely marking a delegated
block as escaped caused the second iteration's break to raise an
error. The test here loops twice, which is sufficient to test
that the block's break continues to work on the n+1 iteration.
    @headius
    headius committed Nov 8, 2017
    Copy the full SHA
    e09a7bb View commit details

  3. Eliminate unnecessary dispatch to shared callBlock logic.

    @headius
    headius committed Nov 8, 2017
    Copy the full SHA
    8e4ba21 View commit details
6 changes: 1 addition & 5 deletions core/src/main/java/org/jruby/ir/instructions/CallBase.java
View file
Original file line number Diff line number Diff line change
@@ -426,11 +426,7 @@ public Object interpret(ThreadContext context, StaticScope currScope, DynamicSco
Block preparedBlock = prepareBlock(context, self, currScope, dynamicScope, temp);

if (hasLiteralClosure()) {
try {
return callSite.call(context, self, object, values, preparedBlock);
} finally {
preparedBlock.escape();
}
return callSite.callIter(context, self, object, values, preparedBlock);
}

return callSite.call(context, self, object, values, preparedBlock);
6 changes: 6 additions & 0 deletions core/src/main/java/org/jruby/ir/instructions/specialized/OneOperandArgBlockCallInstr.java
View file
Original file line number Diff line number Diff line change
@@ -27,9 +27,15 @@ public Instr clone(CloneInfo ii) {

@Override
public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope dynamicScope, IRubyObject self, Object[] temp) {
// NOTE: This logic shouod always match the CALL_10B logic in InterpreterEngine.processCall
IRubyObject object = (IRubyObject) getReceiver().retrieve(context, self, currScope, dynamicScope, temp);
IRubyObject arg1 = (IRubyObject) getArg1().retrieve(context, self, currScope, dynamicScope, temp);
Block preparedBlock = prepareBlock(context, self, currScope, dynamicScope, temp);

if (hasLiteralClosure()) {
return getCallSite().callIter(context, self, object, arg1, preparedBlock);
}

return getCallSite().call(context, self, object, arg1, preparedBlock);
}
}
7 changes: 6 additions & 1 deletion core/src/main/java/org/jruby/ir/interpreter/InterpreterEngine.java
View file
Original file line number Diff line number Diff line change
@@ -57,6 +57,7 @@
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block;
import org.jruby.runtime.CallSite;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.Frame;
import org.jruby.runtime.Helpers;
@@ -325,11 +326,15 @@ protected static void processCall(ThreadContext context, Instr instr, Operation
break;
}
case CALL_1OB: {
// NOTE: This logic shouod always match OneOperandArgBlockCallInstr
OneOperandArgBlockCallInstr call = (OneOperandArgBlockCallInstr)instr;
IRubyObject r = (IRubyObject)retrieveOp(call.getReceiver(), context, self, currDynScope, currScope, temp);
IRubyObject o = (IRubyObject)call.getArg1().retrieve(context, self, currScope, currDynScope, temp);
Block preparedBlock = call.prepareBlock(context, self, currScope, currDynScope, temp);
result = call.getCallSite().callIter(context, self, r, o, preparedBlock);
CallSite callSite = call.getCallSite();
result = call.hasLiteralClosure() ?
callSite.callIter(context, self, r, o, preparedBlock) :
callSite.call(context, self, r, o, preparedBlock);
setResult(temp, currDynScope, call.getResult(), result);
break;
}
70 changes: 25 additions & 45 deletions core/src/main/java/org/jruby/runtime/callsite/CachingCallSite.java
View file
Original file line number Diff line number Diff line change
@@ -69,23 +69,19 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, args, context, self);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
RubyClass selfType = getClass(self);
// This must be retrieved *once* to avoid racing with other threads.
CacheEntry cache = this.cache;
if (CacheEntry.typeOk(cache, selfType)) {
return cache.method.call(context, self, selfType, methodName, args, block);
CacheEntry cache1 = this.cache;
if (CacheEntry.typeOk(cache1, selfType)) {
return cache1.method.call(context, self, selfType, methodName, args, block);
}
return cacheAndCall(caller, selfType, block, args, context, self);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
return callBlock(context, caller, self, args, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
try {
return callBlock(context, caller, self, args, block);
return call(context, caller, self, args, block);
} finally {
block.escape();
}
@@ -131,23 +127,19 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
RubyClass selfType = getClass(self);
// This must be retrieved *once* to avoid racing with other threads.
CacheEntry cache = this.cache;
if (CacheEntry.typeOk(cache, selfType)) {
return cache.method.call(context, self, selfType, methodName, block);
CacheEntry cache1 = this.cache;
if (CacheEntry.typeOk(cache1, selfType)) {
return cache1.method.call(context, self, selfType, methodName, block);
}
return cacheAndCall(caller, selfType, block, context, self);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
return callBlock(context, caller, self, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
try {
return callBlock(context, caller, self, block);
return call(context, caller, self, block);
} finally {
block.escape();
}
@@ -163,23 +155,19 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, arg1);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
RubyClass selfType = getClass(self);
// This must be retrieved *once* to avoid racing with other threads.
CacheEntry cache = this.cache;
if (CacheEntry.typeOk(cache, selfType)) {
return cache.method.call(context, self, selfType, methodName, arg1, block);
CacheEntry cache1 = this.cache;
if (CacheEntry.typeOk(cache1, selfType)) {
return cache1.method.call(context, self, selfType, methodName, arg1, block);
}
return cacheAndCall(caller, selfType, block, context, self, arg1);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
return callBlock(context, caller, self, arg1, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
try {
return callBlock(context, caller, self, arg1, block);
return call(context, caller, self, arg1, block);
} finally {
block.escape();
}
@@ -195,23 +183,19 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, arg1, arg2);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
RubyClass selfType = getClass(self);
// This must be retrieved *once* to avoid racing with other threads.
CacheEntry cache = this.cache;
if (CacheEntry.typeOk(cache, selfType)) {
return cache.method.call(context, self, selfType, methodName, arg1, arg2, block);
CacheEntry cache1 = this.cache;
if (CacheEntry.typeOk(cache1, selfType)) {
return cache1.method.call(context, self, selfType, methodName, arg1, arg2, block);
}
return cacheAndCall(caller, selfType, block, context, self, arg1, arg2);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
return callBlock(context, caller, self, arg1, arg2, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
try {
return callBlock(context, caller, self, arg1, arg2, block);
return call(context, caller, self, arg1, arg2, block);
} finally {
block.escape();
}
@@ -227,23 +211,19 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, arg1, arg2, arg3);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
RubyClass selfType = getClass(self);
// This must be retrieved *once* to avoid racing with other threads.
CacheEntry cache = this.cache;
if (CacheEntry.typeOk(cache, selfType)) {
return cache.method.call(context, self, selfType, methodName, arg1, arg2, arg3, block);
CacheEntry cache1 = this.cache;
if (CacheEntry.typeOk(cache1, selfType)) {
return cache1.method.call(context, self, selfType, methodName, arg1, arg2, arg3, block);
}
return cacheAndCall(caller, selfType, block, context, self, arg1, arg2, arg3);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
return callBlock(context, caller, self, arg1, arg2, arg3, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
try {
return callBlock(context, caller, self, arg1, arg2, arg3, block);
return call(context, caller, self, arg1, arg2, arg3, block);
} finally {
block.escape();
}
40 changes: 10 additions & 30 deletions core/src/main/java/org/jruby/runtime/callsite/SuperCallSite.java
View file
Original file line number Diff line number Diff line change
@@ -55,7 +55,7 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, args, context, self, name);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
RubyModule klazz = context.getFrameKlazz();
String name = context.getFrameName();
RubyClass selfType = pollAndGetClass(context, self, klazz, name);
@@ -67,13 +67,9 @@ private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyOb
return cacheAndCall(caller, selfType, block, args, context, self, name);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
return callBlock(context, caller, self, args, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject[] args, Block block) {
try {
return callBlock(context, caller, self, args, block);
return call(context, caller, self, args, block);
} finally {
block.escape();
}
@@ -121,7 +117,7 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, name);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
RubyModule klazz = context.getFrameKlazz();
String name = context.getFrameName();
RubyClass selfType = pollAndGetClass(context, self, klazz, name);
@@ -133,13 +129,9 @@ private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyOb
return cacheAndCall(caller, selfType, block, context, self, name);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
return callBlock(context, caller, self, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, Block block) {
try {
return callBlock(context, caller, self, block);
return call(context, caller, self, block);
} finally {
block.escape();
}
@@ -157,7 +149,7 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, name, arg1);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
RubyModule klazz = context.getFrameKlazz();
String name = context.getFrameName();
RubyClass selfType = pollAndGetClass(context, self, klazz, name);
@@ -169,13 +161,9 @@ private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyOb
return cacheAndCall(caller, selfType, block, context, self, name, arg1);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
return callBlock(context, caller, self, arg1, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, Block block) {
try {
return callBlock(context, caller, self, arg1, block);
return call(context, caller, self, arg1, block);
} finally {
block.escape();
}
@@ -193,7 +181,7 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, name, arg1, arg2);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
RubyModule klazz = context.getFrameKlazz();
String name = context.getFrameName();
RubyClass selfType = pollAndGetClass(context, self, klazz, name);
@@ -205,13 +193,9 @@ private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyOb
return cacheAndCall(caller, selfType, block, context, self, name, arg1, arg2);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
return callBlock(context, caller, self, arg1, arg2, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, Block block) {
try {
return callBlock(context, caller, self, arg1, arg2, block);
return call(context, caller, self, arg1, arg2, block);
} finally {
block.escape();
}
@@ -229,7 +213,7 @@ public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject s
return cacheAndCall(caller, selfType, context, self, name, arg1, arg2, arg3);
}

private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
RubyModule klazz = context.getFrameKlazz();
String name = context.getFrameName();
RubyClass selfType = pollAndGetClass(context, self, klazz, name);
@@ -241,13 +225,9 @@ private IRubyObject callBlock(ThreadContext context, IRubyObject caller, IRubyOb
return cacheAndCall(caller, selfType, block, context, self, name, arg1, arg2, arg3);
}

public IRubyObject call(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
return callBlock(context, caller, self, arg1, arg2, arg3, block);
}

public IRubyObject callIter(ThreadContext context, IRubyObject caller, IRubyObject self, IRubyObject arg1, IRubyObject arg2, IRubyObject arg3, Block block) {
try {
return callBlock(context, caller, self, arg1, arg2, arg3, block);
return call(context, caller, self, arg1, arg2, arg3, block);
} finally {
block.escape();
}
18 changes: 18 additions & 0 deletions spec/ruby/language/break_spec.rb
View file
Original file line number Diff line number Diff line change
@@ -24,6 +24,24 @@
value.should == :value
end
end

describe "captured and delegated to another method repeatedly" do
it "breaks out of the block" do
@program.looped_break_in_captured_block
ScratchPad.recorded.should == [:begin,
:preloop,
:predele,
:preyield,
:prebreak,
:postbreak,
:postyield,
:postdele,
:predele,
:preyield,
:prebreak,
:end]
end
end
end

describe "The break statement in a captured block" do
28 changes: 28 additions & 0 deletions spec/ruby/language/fixtures/break.rb
View file
Original file line number Diff line number Diff line change
@@ -106,6 +106,34 @@ def break_in_yielding_method
note :d
end

def looped_break_in_captured_block
note :begin
looped_delegate_block do |i|
note :prebreak
break if i == 1
note :postbreak
end
note :end
end

def looped_delegate_block(&block)
note :preloop
2.times do |i|
note :predele
yield_value(i, &block)
note :postdele
end
note :postloop
end
private :looped_delegate_block

def yield_value(value)
note :preyield
yield value
note :postyield
end
private :yield_value

def method(v)
yield v
end