Skip to content
Browse files

[IR] Removed 'container' field from IRScope classes since it was a bad

design.  I had initially added it way back assuming it was known
statically and changed it to an Operand type later on when I realized
it was not known statically.  This since got in the way of code
transformations because IRScope is never an operand in instructions and
Operand renames/manipulations had to remember to keep this Operand field
in IR* classes consistent. Refactored code to get rid of all the places
it was used.
  • Loading branch information...
1 parent a7c4398 commit 1f4e8fc537dd47781173f823df0cd2e7f383bfa3 @subbuss subbuss committed Oct 13, 2011
View
21 src/org/jruby/compiler/ir/IRBuilder.java
@@ -170,7 +170,6 @@
import org.jruby.compiler.ir.operands.BacktickString;
import org.jruby.compiler.ir.operands.Bignum;
import org.jruby.compiler.ir.operands.BooleanLiteral;
-import org.jruby.compiler.ir.operands.ClassMetaObject;
import org.jruby.compiler.ir.operands.CompoundArray;
import org.jruby.compiler.ir.operands.CompoundString;
import org.jruby.compiler.ir.operands.DynamicSymbol;
@@ -183,7 +182,6 @@
import org.jruby.compiler.ir.operands.LocalVariable;
import org.jruby.compiler.ir.operands.MetaObject;
import org.jruby.compiler.ir.operands.MethAddr;
-import org.jruby.compiler.ir.operands.ModuleMetaObject;
import org.jruby.compiler.ir.operands.Nil;
import org.jruby.compiler.ir.operands.UnexecutableNil;
import org.jruby.compiler.ir.operands.NthRef;
@@ -1023,10 +1021,9 @@ public Operand buildClass(ClassNode classNode, IRScope s) {
String className = cpath.getName();
Operand container = getContainerFromCPath(cpath, s);
- IRClass c = new IRClass(s, container, superClass, className, classNode.getScope());
- ClassMetaObject cmo = (ClassMetaObject) MetaObject.create(c);
+ IRClass c = new IRClass(s, superClass, className, classNode.getScope());
Variable ret = s.getNewTemporaryVariable();
- s.addInstr(new DefineClassInstr(ret, cmo, superClass));
+ s.addInstr(new DefineClassInstr(ret, c, container, superClass));
s.getNearestModule().addClass(c);
IRMethod rootMethod = c.getRootMethod();
@@ -1708,8 +1705,8 @@ public Operand buildDAsgn(final DAsgnNode dasgnNode, IRScope s) {
// because of the use of copyAndReturnValue method for literal objects.
}
- private IRMethod defineNewMethod(MethodDefNode defNode, IRScope s, Operand container, boolean isInstanceMethod) {
- IRMethod method = new IRMethod(s, container, defNode.getName(), isInstanceMethod, defNode.getScope());
+ private IRMethod defineNewMethod(MethodDefNode defNode, IRScope s, boolean isInstanceMethod) {
+ IRMethod method = new IRMethod(s, defNode.getName(), isInstanceMethod, defNode.getScope());
// Build IR for arguments
receiveArgs(defNode.getArgsNode(), method);
@@ -1736,20 +1733,20 @@ public Operand buildDefn(MethodDefNode node, IRScope s) { // Instance method
// DefineIstanceMethod IR interpretation currently relies on this static determination for handling top-level methods
if ((s instanceof IRMethod) && ((IRMethod)s).isAModuleRootMethod()) {
container = MetaObject.create(s.getNearestModule());
- method = defineNewMethod(node, s, container, true);
+ method = defineNewMethod(node, s, true);
s.getNearestModule().addMethod(method);
}
else {
container = getSelf(s);
- method = defineNewMethod(node, s, container, true);
+ method = defineNewMethod(node, s, true);
}
s.addInstr(new DefineInstanceMethodInstr(container, method));
return Nil.NIL;
}
public Operand buildDefs(DefsNode node, IRScope s) { // Class method
Operand container = build(node.getReceiverNode(), s);
- IRMethod method = defineNewMethod(node, s, container, false);
+ IRMethod method = defineNewMethod(node, s, false);
// ENEBO: Can all metaobjects be used for this? closure?
//if (container instanceof MetaObject) {
// ((IRModule) ((MetaObject) container).getScope()).addMethod(method);
@@ -2392,9 +2389,9 @@ public Operand buildModule(ModuleNode moduleNode, IRScope s) {
Operand container = getContainerFromCPath(cpath, s);
// Build the new module
- IRModule m = new IRModule(s, container, moduleName, moduleNode.getScope());
+ IRModule m = new IRModule(s, moduleName, moduleNode.getScope());
Variable ret = s.getNewTemporaryVariable();
- s.addInstr(new DefineModuleInstr(ret, (ModuleMetaObject) MetaObject.create(m)));
+ s.addInstr(new DefineModuleInstr(m, ret, container));
s.getNearestModule().addModule(m);
IRMethod rootMethod = m.getRootMethod();
View
4 src/org/jruby/compiler/ir/IRClass.java
@@ -6,8 +6,8 @@
public class IRClass extends IRModule {
final public Operand superClass;
- public IRClass(IRScope lexicalParent, Operand container, Operand superClass, String className, StaticScope staticScope) {
- super(lexicalParent, container, className, staticScope);
+ public IRClass(IRScope lexicalParent, Operand superClass, String className, StaticScope staticScope) {
+ super(lexicalParent, className, staticScope);
this.superClass = superClass;
}
View
4 src/org/jruby/compiler/ir/IRClosure.java
@@ -43,7 +43,7 @@
private List<Operand> blockArgs;
public IRClosure(IRScope lexicalParent, boolean isForLoopBody, StaticScope staticScope, Arity arity, int argumentType) {
- super(lexicalParent, MetaObject.create(lexicalParent), null, staticScope);
+ super(lexicalParent, null, staticScope);
this.isForLoopBody = isForLoopBody;
String prefix = isForLoopBody ? "_FOR_LOOP_" : "_CLOSURE_";
this.startLabel = getNewLabel(prefix + "START");
@@ -63,7 +63,7 @@ public IRClosure(IRScope lexicalParent, boolean isForLoopBody, StaticScope stati
@Override
public int getNextClosureId() {
- return lexicalParent.getNextClosureId();
+ return getLexicalParent().getNextClosureId();
}
@Override
View
4 src/org/jruby/compiler/ir/IRExecutionScope.java
@@ -110,8 +110,8 @@ private void init() {
nextLocalVariableSlot = 0;
}
- public IRExecutionScope(IRScope lexicalParent, Operand container, String name, StaticScope staticScope) {
- super(lexicalParent, container, name, staticScope);
+ public IRExecutionScope(IRScope lexicalParent, String name, StaticScope staticScope) {
+ super(lexicalParent, name, staticScope);
init();
}
View
3 src/org/jruby/compiler/ir/IRMetaClass.java
@@ -10,8 +10,7 @@
public IRMetaClass(IRScope s, StaticScope staticScope) {
// Super class is always <Class:Class>
- // This metaclass is always top-level, hence the null container.
- super(s, null, MetaObject.create(CLASS_METACLASS), "<DUMMY_MC:" + dummyMetaClassCount + ">", staticScope);
+ super(s, MetaObject.create(CLASS_METACLASS), "<DUMMY_MC:" + dummyMetaClassCount + ">", staticScope);
dummyMetaClassCount += 1;
}
View
4 src/org/jruby/compiler/ir/IRMethod.java
@@ -34,8 +34,8 @@
private int nextAvailableBindingSlot;
private Map<String, Integer> bindingSlotMap;
- public IRMethod(IRScope lexicalParent, Operand container, String name, boolean isInstanceMethod, StaticScope staticScope) {
- super(lexicalParent, container, name, staticScope);
+ public IRMethod(IRScope lexicalParent, String name, boolean isInstanceMethod, StaticScope staticScope) {
+ super(lexicalParent, name, staticScope);
this.isInstanceMethod = isInstanceMethod;
startLabel = getNewLabel("_METH_START");
endLabel = getNewLabel("_METH_END");
View
12 src/org/jruby/compiler/ir/IRModule.java
@@ -44,11 +44,11 @@
}
static private IRClass addCoreClass(String name, IRScope parent, String[] coreMethods, StaticScope staticScope) {
- IRClass c = new IRClass(parent, null, null, name, staticScope);
+ IRClass c = new IRClass(parent, null, name, staticScope);
coreClasses.put(c.getName(), c);
if (coreMethods != null) {
for (String m : coreMethods) {
- IRMethod meth = new IRMethod(c, null, m, true, null);
+ IRMethod meth = new IRMethod(c, m, true, null);
meth.setCodeModificationFlag(false);
c.addMethod(meth);
}
@@ -99,7 +99,7 @@ private void addRootMethod() {
// end
//
String n = ROOT_METHOD_PREFIX + getName();
- rootMethod = new IRMethod(this, MetaObject.create(this), n, false, IRStaticScopeFactory.newIRLocalScope(rootObjectScope));
+ rootMethod = new IRMethod(this, n, false, IRStaticScopeFactory.newIRLocalScope(rootObjectScope));
rootMethod.addInstr(new ReceiveSelfInstruction(rootMethod.getSelf())); // Set up self!
rootMethod.addInstr(new ReceiveClosureInstr(rootMethod.getImplicitBlockArg()));
}
@@ -208,10 +208,8 @@ public IRModule getNearestModule() {
return this;
}
- public IRModule(IRScope lexicalParent, Operand container, String name, StaticScope scope) {
- // SSS FIXME: container could be a meta-object which means we can record the constant statically!
- // Add in this opt!
- super(lexicalParent, container, name, scope);
+ public IRModule(IRScope lexicalParent, String name, StaticScope scope) {
+ super(lexicalParent, name, scope);
addRootMethod();
updateVersion();
}
View
5 src/org/jruby/compiler/ir/IRScope.java
@@ -16,11 +16,6 @@
*/
public interface IRScope {
/**
- * Returns the containing parent scope
- */
- public Operand getContainer();
-
- /**
* Returns the lexical scope that contains this scope definition
*/
public IRScope getLexicalParent();
View
19 src/org/jruby/compiler/ir/IRScopeImpl.java
@@ -61,10 +61,8 @@
public abstract class IRScopeImpl implements IRScope {
private static final Logger LOG = LoggerFactory.getLogger("IRScope");
- // SSS FIXME: Dumb design leaking a live operand into a non-operand!!
- Operand container; // Parent container for this context
- RubyModule containerModule; // Live version of container
- IRScope lexicalParent; // Lexical parent scope
+ private IRScope lexicalParent; // Lexical parent scope
+ private RubyModule containerModule; // Live version of container
private String name;
@@ -82,23 +80,12 @@
private StaticScope staticScope;
- public IRScopeImpl(IRScope lexicalParent, Operand container, String name, StaticScope staticScope) {
+ public IRScopeImpl(IRScope lexicalParent, String name, StaticScope staticScope) {
this.lexicalParent = lexicalParent;
- this.container = container;
this.name = name;
this.staticScope = staticScope;
}
- // Update the containing scope
- public void setContainer(Operand o) {
- container = o;
- }
-
- // Returns the containing scope!
- public Operand getContainer() {
- return container;
- }
-
public RubyModule getContainerModule() {
// System.out.println("GET: container module of " + getName() + " with hc " + hashCode() + " to " + containerModule.getName());
return containerModule;
View
4 src/org/jruby/compiler/ir/IRScript.java
@@ -10,8 +10,8 @@
private final IRClass dummyClass; // Dummy class for the script
public IRScript(String className, String sourceName, StaticScope staticScope) {
- super((IRScope) null, null, sourceName, staticScope);
- dummyClass = new IRClass(this, null, null, "[script]:" + sourceName, staticScope);
+ super((IRScope) null, sourceName, staticScope);
+ dummyClass = new IRClass(this, null, "[script]:" + sourceName, staticScope);
}
public Operand getFileName() {
View
37 src/org/jruby/compiler/ir/instructions/DefineClassInstr.java
@@ -4,8 +4,8 @@
import org.jruby.RubyModule;
import org.jruby.RubyClass;
import org.jruby.compiler.ir.IRScope;
+import org.jruby.compiler.ir.IRClass;
import org.jruby.compiler.ir.IRMetaClass;
-import org.jruby.compiler.ir.operands.ClassMetaObject;
import org.jruby.compiler.ir.operands.Label;
import org.jruby.compiler.ir.operands.Nil;
import org.jruby.compiler.ir.operands.Operand;
@@ -15,27 +15,32 @@
import org.jruby.interpreter.InterpreterContext;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.internal.runtime.methods.InterpretedIRMethod;
public class DefineClassInstr extends TwoOperandInstr {
- public DefineClassInstr(Variable dest, ClassMetaObject cmo, Operand superClass) {
+ IRClass newIRClass;
+ public DefineClassInstr(Variable dest, IRClass newIRClass, Operand container, Operand superClass) {
// Get rid of null scenario
- super(Operation.DEF_CLASS, dest, cmo, superClass == null ? Nil.NIL : superClass);
+ super(Operation.DEF_CLASS, dest, container, superClass == null ? Nil.NIL : superClass);
+ this.newIRClass = newIRClass;
}
@Override
public Instr cloneForInlining(InlinerInfo ii) {
- return new DefineClassInstr(ii.getRenamedVariable(result), (ClassMetaObject)getOperand1(), getOperand2().cloneForInlining(ii));
+ return new DefineClassInstr(ii.getRenamedVariable(result), this.newIRClass, getOperand1().cloneForInlining(ii), getOperand2().cloneForInlining(ii));
}
@Override
public Label interpret(InterpreterContext interp, ThreadContext context, IRubyObject self) {
- Ruby runtime = context.getRuntime();
- ClassMetaObject cmo = (ClassMetaObject) getOperand1();
- IRScope scope = cmo.scope;
- RubyModule container = cmo.getContainer(interp, context, self);
- RubyModule module;
- if (scope instanceof IRMetaClass) {
- module = container.getMetaClass();
+ RubyModule classContainer = null;
+ Object c = getOperand1().retrieve(interp, context, self);
+ if (c instanceof RubyModule) classContainer = (RubyModule)c;
+ else throw context.getRuntime().newTypeError("no outer class/module");
+
+ RubyModule newRubyClass;
+ if (newIRClass instanceof IRMetaClass) {
+ newRubyClass = classContainer.getMetaClass();
} else {
Operand superClass = getOperand2();
RubyClass sc;
@@ -47,10 +52,16 @@ public Label interpret(InterpreterContext interp, ThreadContext context, IRubyOb
if (o instanceof RubyClass) sc = (RubyClass)o;
else throw context.getRuntime().newTypeError("superclass must be Class (" + o + " given)");
}
- module = container.defineOrGetClassUnder(scope.getName(), sc);
+ newRubyClass = classContainer.defineOrGetClassUnder(newIRClass.getName(), sc);
}
- Object v = cmo.interpretBody(interp, context, module);
+ // Interpret the body
+ newIRClass.getStaticScope().setModule(newRubyClass);
+ DynamicMethod method = new InterpretedIRMethod(newIRClass.getRootMethod(), newRubyClass);
+ // SSS FIXME: Rather than pass the block implicitly, should we add %block as another operand to DefineClass, DefineModule instrs?
+ Object v = method.call(context, newRubyClass, newRubyClass, "", new IRubyObject[]{}, interp.getBlock());
+
+ // Result from interpreting the body
getResult().store(interp, context, self, v);
return null;
}
View
4 src/org/jruby/compiler/ir/instructions/DefineClassMethodInstr.java
@@ -40,10 +40,6 @@ public Instr cloneForInlining(InlinerInfo ii) {
@Override
public void simplifyOperands(Map<Operand, Operand> valueMap) {
super.simplifyOperands(valueMap);
- Operand v = valueMap.get(getArg());
- // SSS FIXME: Dumb design leaking operand into IRScopeImpl -- hence this setting going on here. Fix it!
- if (v != null)
- method.setContainer(v);
}
// SSS FIXME: Go through this and DefineInstanceMethodInstr.interpret, clean up, extract common code
View
4 src/org/jruby/compiler/ir/instructions/DefineInstanceMethodInstr.java
@@ -48,10 +48,6 @@ public Instr cloneForInlining(InlinerInfo ii) {
@Override
public void simplifyOperands(Map<Operand, Operand> valueMap) {
super.simplifyOperands(valueMap);
- Operand v = valueMap.get(getArg());
- // SSS FIXME: Dumb design leaking operand into IRScopeImpl -- hence this setting going on here. Fix it!
- if (v != null)
- method.setContainer(v);
}
// SSS FIXME: Go through this and DefineClassMethodInstr.interpret, clean up, extract common code
View
32 src/org/jruby/compiler/ir/instructions/DefineModuleInstr.java
@@ -3,33 +3,45 @@
import org.jruby.Ruby;
import org.jruby.RubyModule;
import org.jruby.compiler.ir.IRScope;
+import org.jruby.compiler.ir.IRModule;
import org.jruby.compiler.ir.operands.Label;
-import org.jruby.compiler.ir.operands.ModuleMetaObject;
+import org.jruby.compiler.ir.operands.Operand;
import org.jruby.compiler.ir.operands.Variable;
import org.jruby.compiler.ir.Operation;
import org.jruby.compiler.ir.representations.InlinerInfo;
import org.jruby.interpreter.InterpreterContext;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.internal.runtime.methods.InterpretedIRMethod;
public class DefineModuleInstr extends OneOperandInstr {
- public DefineModuleInstr(Variable dest, ModuleMetaObject m) {
- super(Operation.DEF_MODULE, dest, m);
+ public final IRModule newIRModule;
+
+ public DefineModuleInstr(IRModule newIRModule, Variable dest, Operand container) {
+ super(Operation.DEF_MODULE, dest, container);
+ this.newIRModule = newIRModule;
}
@Override
public Instr cloneForInlining(InlinerInfo ii) {
- return new DefineModuleInstr(ii.getRenamedVariable(result), (ModuleMetaObject)getArg());
+ return new DefineModuleInstr(this.newIRModule, ii.getRenamedVariable(result), getArg().cloneForInlining(ii));
}
@Override
public Label interpret(InterpreterContext interp, ThreadContext context, IRubyObject self) {
- Ruby runtime = context.getRuntime();
- ModuleMetaObject mmo = (ModuleMetaObject)getArg();
- IRScope scope = mmo.scope;
- RubyModule container = mmo.getContainer(interp, context, self);
- RubyModule module = container.defineOrGetModuleUnder(scope.getName());
- getResult().store(interp, context, self, mmo.interpretBody(interp, context, module));
+ RubyModule moduleContainer = null;
+ Object c = getArg().retrieve(interp, context, self);
+ if (c instanceof RubyModule) moduleContainer = (RubyModule)c;
+ else throw context.getRuntime().newTypeError("no outer class/module");
+
+ RubyModule newRubyModule = moduleContainer.defineOrGetModuleUnder(newIRModule.getName());
+ newIRModule.getStaticScope().setModule(newRubyModule);
+ DynamicMethod method = new InterpretedIRMethod(newIRModule.getRootMethod(), newRubyModule);
+
+ // SSS FIXME: Rather than pass the block implicitly, should we add %block as another operand to DefineClass, DefineModule instrs?
+ Object value = method.call(context, newRubyModule, newRubyModule, "", new IRubyObject[]{}, interp.getBlock());
+ getResult().store(interp, context, self, value);
return null;
}
}
View
32 src/org/jruby/compiler/ir/operands/MetaObject.java
@@ -9,11 +9,6 @@
import org.jruby.compiler.ir.IRModule;
import org.jruby.compiler.ir.IRMethod;
import org.jruby.compiler.ir.IRScope;
-import org.jruby.internal.runtime.methods.DynamicMethod;
-import org.jruby.internal.runtime.methods.InterpretedIRMethod;
-import org.jruby.interpreter.InterpreterContext;
-import org.jruby.runtime.ThreadContext;
-import org.jruby.runtime.builtin.IRubyObject;
public class MetaObject extends Operand {
public final IRScope scope;
@@ -60,36 +55,9 @@ public IRScope getScope() {
return scope;
}
- @Override
- public void addUsedVariables(List<Variable> l) {
- // SSS: IRScopeImpl has an operand that records the container for the scope
- Operand c = scope.getContainer();
- if (c != null) c.addUsedVariables(l);
- }
-
// SSS FIXME: Incomplete!
@Override
public IRClass getTargetClass() {
return (scope instanceof IRModule) ? IRClass.getCoreClass("Module") : null;
}
-
- public Object interpretBody(InterpreterContext interp, ThreadContext context, RubyModule module) {
- scope.getStaticScope().setModule(module);
- IRMethod rootMethod = ((IRModule) scope).getRootMethod();
- DynamicMethod method = new InterpretedIRMethod(rootMethod, module);
-
- // SSS FIXME: Rather than pass the block implicitly, should we add %block as another operand to DefineClass, DefineModule instrs?
- return method.call(context, module, module, "", new IRubyObject[]{}, interp.getBlock());
- }
-
- public RubyModule getContainer(InterpreterContext interp, ThreadContext context, IRubyObject self) {
- if (scope.getContainer() == null) {
- return context.getRuntime().getObject();
- }
- else {
- Object c = scope.getContainer().retrieve(interp, context, self);
- if (c instanceof RubyModule) return (RubyModule)c;
- else throw context.getRuntime().newTypeError("no outer class/module");
- }
- }
}
View
11 src/org/jruby/compiler/ir/operands/ModuleMetaObject.java
@@ -33,15 +33,4 @@ else if (module.isACoreClass()) {
return null;
}
}
-
- @Override
- public Object store(InterpreterContext interp, ThreadContext context, IRubyObject self, Object value) {
- // Store it in live tree of modules/classes
- RubyModule container = (RubyModule) scope.getContainer().retrieve(interp, context, self);
- container.setConstant(scope.getName(), (RubyModule) value);
-
- // Save reference into scope for easy access
- scope.getStaticScope().setModule((RubyModule) value);
- return (RubyModule) value;
- }
}
View
4 src/org/jruby/compiler/ir/operands/Operand.java
@@ -65,11 +65,11 @@ public Operand cloneForInlining(InlinerInfo ii) {
@Interp
public Object retrieve(InterpreterContext interp, ThreadContext context, IRubyObject self) {
- throw new RuntimeException(this.getClass().getSimpleName() + " should not be directly interpreted");
+ throw new RuntimeException(this.getClass().getSimpleName() + " should not be directly retrieved.");
}
@Interp
public Object store(InterpreterContext interp, ThreadContext context, IRubyObject self, Object value) {
- throw new RuntimeException(this.getClass().getSimpleName() + " should not be directly interpreted");
+ throw new RuntimeException(this.getClass().getSimpleName() + " should not be directly stored.");
}
}

0 comments on commit 1f4e8fc

Please sign in to comment.
Something went wrong with that request. Please try again.