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: 01a3f6851ba6
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: debee1d64d9b
Choose a head ref
  • 2 commits
  • 4 files changed
  • 1 contributor

Commits on Jun 30, 2015

  1. Revert "Fixes #3056. java.lang.IncompatibleClassChangeError in 9.0.0.… 

    …0 rc1"

This fixed the issue but @subbuss rightly points out this scope should have
been marked as BINDING_HAS_ESCAPED and thus this pass should not have even
ran.

This reverts commit 467e2f7.
    @enebo
    enebo committed Jun 30, 2015
    Copy the full SHA
    11bbc8a View commit details

  2. Fixes #3056. java.lang.IncompatibleClassChangeError in 9.0.0.0 rc1. 

    This replaces the previous fix.  The previous fix was wrong because the
code in question contained a closure and we were not marking the parent
method as scope as BINDING_HAS_ESCAPED.  As a result, OptimizeDelegationPass
was executing and mucking things up.  The previous fix corrected the pass
to work with nested closures but it would create new problems if that closure
did something which needed to conume the parents binding.

The new fix is just a simple oversight in a few of our instructions.  ZSuper,
UnresolvesSuper, and Match are calls but they were not calling their
superclasses computeScopeFlags.  This is turn made the issue at hand not notice
that the method contained a closure so it never marked BINDING_HAS_ESCAPED.

This fix should potentially fix other weird errors where we did not set common
call IRScope flags.
    @enebo
    enebo committed Jun 30, 2015
    Copy the full SHA
    debee1d View commit details
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/instructions/MatchInstr.java
View file
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ public MatchInstr(Variable result, Operand receiver, Operand arg) {

@Override
public boolean computeScopeFlags(IRScope scope) {
super.computeScopeFlags(scope);
// $~ is implicitly used since Backref and NthRef operands
// access it and $~ is not made explicit in those operands.
scope.getFlags().add(USES_BACKREF_OR_LASTLINE);
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/instructions/UnresolvedSuperInstr.java
View file
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ public UnresolvedSuperInstr(Variable result, Operand receiver, Operand[] args, O

@Override
public boolean computeScopeFlags(IRScope scope) {
super.computeScopeFlags(scope);
scope.getFlags().add(IRFlags.REQUIRES_FRAME); // for current class and method name
scope.getFlags().add(IRFlags.REQUIRES_DYNSCOPE); // for current class and method name
return true;
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/instructions/ZSuperInstr.java
View file
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ public ZSuperInstr(Variable result, Operand receiver, Operand[] args, Operand cl

@Override
public boolean computeScopeFlags(IRScope scope) {
super.computeScopeFlags(scope);
scope.getFlags().add(IRFlags.USES_ZSUPER);
scope.getFlags().add(IRFlags.CAN_CAPTURE_CALLERS_BINDING);
return true;
39 changes: 11 additions & 28 deletions core/src/main/java/org/jruby/ir/passes/OptimizeDelegationPass.java
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.jruby.ir.passes;

import org.jruby.ir.IRClosure;
import org.jruby.ir.IRScope;
import org.jruby.ir.IRFlags;
import org.jruby.ir.instructions.*;
import org.jruby.ir.instructions.ClosureAcceptingInstr;
import org.jruby.ir.instructions.CopyInstr;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.instructions.ReifyClosureInstr;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.representations.BasicBlock;
@@ -32,7 +34,9 @@ public boolean invalidate(IRScope s) {
return false;
}

private static void calculateBlockUsage(IRScope s, Map<Operand, Operand> unusedExplicitBlocks) {
private static void optimizeDelegatedVars(IRScope s) {
Map<Operand, Operand> unusedExplicitBlocks = new HashMap<>();

for (BasicBlock bb: s.getCFG().getBasicBlocks()) {
for (Instr i: bb.getInstrs()) {
if (i instanceof ReifyClosureInstr) {
@@ -50,12 +54,6 @@ private static void calculateBlockUsage(IRScope s, Map<Operand, Operand> unusedE
}
}

for (IRScope child: s.getClosures()) {
calculateBlockUsage(child, unusedExplicitBlocks);
}
}

private static void replaceBlockUsage(IRScope s, Map<Operand, Operand> unusedExplicitBlocks) {
for (BasicBlock bb: s.getCFG().getBasicBlocks()) {
ListIterator<Instr> instrs = bb.getInstrs().listIterator();
while (instrs.hasNext()) {
@@ -72,28 +70,13 @@ private static void replaceBlockUsage(IRScope s, Map<Operand, Operand> unusedExp
}
}
}

for (IRScope child: s.getClosures()) {
replaceBlockUsage(child, unusedExplicitBlocks);
}
}
private static void optimizeDelegatedVars(IRScope s) {
Map<Operand, Operand> unusedExplicitBlocks = new HashMap<>();

calculateBlockUsage(s, unusedExplicitBlocks);
replaceBlockUsage(s, unusedExplicitBlocks);
}

private static boolean usesVariableAsNonClosureArg(Instr i, Variable v) {
List<Variable> usedVariables = i.getUsedVariables();
if (usedVariables.contains(v)) {
if (i instanceof ClosureAcceptingInstr) {
return usedVariables.indexOf(v) != usedVariables.lastIndexOf(v) ||
v != ((ClosureAcceptingInstr) i).getClosureArg();
} else {
return true;
}
}
return false;
return usedVariables.contains(v) &&
(!(i instanceof ClosureAcceptingInstr) ||
usedVariables.indexOf(v) != usedVariables.lastIndexOf(v) ||
v != ((ClosureAcceptingInstr) i).getClosureArg());
}
}