Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix a bunch of issues with cloning.
* Caught while trying to use cloned instructions in interpreter.
  So far, cloning was only being used for ensure-blocks and
  in IRBuilder where these bugs were less problematic or at least
  never caused problems so far.
  • Loading branch information
subbuss committed Oct 11, 2014
1 parent 7a8bf5f commit c02df12
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 11 deletions.
4 changes: 3 additions & 1 deletion core/src/main/java/org/jruby/ir/IRClosure.java
Expand Up @@ -285,7 +285,9 @@ public int getNestingDepth() {

protected IRClosure cloneForInlining(CloneInfo ii, IRClosure clone) {
clone.nestingDepth = this.nestingDepth;
clone.parameterList = this.parameterList;
// FIXME: This is fragile. Untangle this state.
// Why is this being copied over to InterpretedIRBlockBody?
clone.setParameterList(this.parameterList);

SimpleCloneInfo clonedII = ii.cloneForCloningClosure(clone);

Expand Down
4 changes: 1 addition & 3 deletions core/src/main/java/org/jruby/ir/instructions/Instr.java
Expand Up @@ -177,9 +177,7 @@ public void renameVars(Map<Operand, Operand> renameMap) {
* args and return values.
* @return a new instruction that can be used in the target scope.
*/
public Instr clone(CloneInfo info) {
throw new RuntimeException("clone: Not implemented for: " + this.getOperation());
}
public abstract Instr clone(CloneInfo info);

/**
* This method takes as input a map of operands to their values, and outputs
Expand Down
Expand Up @@ -29,8 +29,8 @@ public String toString() {

@Override
public Instr clone(CloneInfo ii) {
// SSS FIXME: This is buggy! 'scope' might have changed because of cloning.
return this;
// Use original scope even if inlined -- so debugging / stack-traces are meaningful
return new LineNumberInstr(scope, lineNumber);
}

@Override
Expand Down
Expand Up @@ -54,7 +54,7 @@ public int getRest() {

@Override
public Instr clone(CloneInfo ii) {
return this;
return new RaiseArgumentErrorInstr(required, opt, rest, numArgs);
}

@Override
Expand Down
Expand Up @@ -3,6 +3,7 @@
import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.transformations.inlining.CloneInfo;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ThreadContext;
Expand All @@ -25,6 +26,11 @@ public Operand[] getOperands() {
return new Operand[0];
}

@Override
public Instr clone(CloneInfo ii) {
return new RaiseRequiredKeywordArgumentError(name);
}

@Override
public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
throw context.runtime.newArgumentError("missing keyword: " + name);
Expand Down
Expand Up @@ -26,10 +26,10 @@ public Operand[] getOperands() {

@Override
public Instr clone(CloneInfo ii) {
if (ii instanceof SimpleCloneInfo) return this;
if (ii instanceof SimpleCloneInfo) return new ThreadPollInstr(onBackEdge);

// Get rid of non-back-edge thread-poll instructions when scopes are inlined
return onBackEdge ? this : null;
return onBackEdge ? new ThreadPollInstr(onBackEdge) : null;
}

@Override
Expand Down
Expand Up @@ -35,7 +35,7 @@ public void addUsedVariables(List<Variable> l) {
}

public Operand cloneForInlining(CloneInfo ii) {
return symbolName.cloneForInlining(ii);
return new DynamicSymbol(symbolName.cloneForInlining(ii));
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/jruby/ir/operands/Label.java
Expand Up @@ -50,7 +50,9 @@ public boolean equals(Object o) {
}

public Label clone() {
return new Label(prefix, id);
Label newL = new Label(prefix, id);
newL.setTargetPC(getTargetPC()); // Strictly not necessary, but, copy everything over
return newL;
}

@Override
Expand Down

0 comments on commit c02df12

Please sign in to comment.