Skip to content

Commit

Permalink
[Truffle] Capture the block in singleton class definitions.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Jan 6, 2016
1 parent 18c78f2 commit 71ae5d4
Showing 6 changed files with 51 additions and 23 deletions.
1 change: 0 additions & 1 deletion spec/truffle/tags/language/class_tags.txt
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
fails:A class definition extending an object (sclass) can use return to cause the enclosing method to return
fails:A class definition extending an object (sclass) allows accessing the block of the original scope
18 changes: 15 additions & 3 deletions truffle/src/main/java/org/jruby/truffle/nodes/methods/MethodDefinitionNode.java
View file
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@
import com.oracle.truffle.api.source.SourceSection;
import org.jruby.runtime.Visibility;
import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.runtime.RubyArguments;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.methods.InternalMethod;
import org.jruby.truffle.runtime.methods.SharedMethodInfo;
@@ -28,21 +29,31 @@ public class MethodDefinitionNode extends RubyNode {

private final String name;
private final SharedMethodInfo sharedMethodInfo;

private final CallTarget callTarget;
private final boolean captureBlock;

public MethodDefinitionNode(RubyContext context, SourceSection sourceSection, String name, SharedMethodInfo sharedMethodInfo,
CallTarget callTarget) {
CallTarget callTarget, boolean captureBlock) {
super(context, sourceSection);
this.name = name;
this.sharedMethodInfo = sharedMethodInfo;
this.callTarget = callTarget;
this.captureBlock = captureBlock;
}

public InternalMethod executeMethod(VirtualFrame frame) {
final DynamicObject dummyModule = getContext().getCoreLibrary().getObjectClass();
final Visibility dummyVisibility = Visibility.PUBLIC;
return new InternalMethod(sharedMethodInfo, name, dummyModule, dummyVisibility, callTarget);

final DynamicObject capturedBlock;

if (captureBlock) {
capturedBlock = RubyArguments.getBlock(frame.getArguments());
} else {
capturedBlock = null;
}

return new InternalMethod(sharedMethodInfo, name, dummyModule, dummyVisibility, false, null, callTarget, capturedBlock);
}

@Override
@@ -57,4 +68,5 @@ public String getName() {
public SharedMethodInfo getSharedMethodInfo() {
return sharedMethodInfo;
}

}
9 changes: 7 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/nodes/yield/YieldNode.java
View file
Original file line number Diff line number Diff line change
@@ -49,11 +49,16 @@ public final Object execute(VirtualFrame frame) {
argumentsObjects[i] = arguments[i].execute(frame);
}

final DynamicObject block = RubyArguments.getBlock(frame.getArguments());
DynamicObject block = RubyArguments.getBlock(frame.getArguments());

if (block == null) {
CompilerDirectives.transferToInterpreter();
throw new RaiseException(getContext().getCoreLibrary().noBlockToYieldTo(this));

block = RubyArguments.getMethod(frame.getArguments()).getCapturedBlock();

if (block == null) {
throw new RaiseException(getContext().getCoreLibrary().noBlockToYieldTo(this));
}
}

if (unsplat) {
25 changes: 17 additions & 8 deletions truffle/src/main/java/org/jruby/truffle/runtime/methods/InternalMethod.java
View file
Original file line number Diff line number Diff line change
@@ -38,19 +38,24 @@ public class InternalMethod implements ObjectGraphNode {
private final DynamicObject proc; // only if method is created from a Proc

private final CallTarget callTarget;
private final DynamicObject capturedBlock;

public static InternalMethod fromProc(SharedMethodInfo sharedMethodInfo, String name, DynamicObject declaringModule,
Visibility visibility, DynamicObject proc, CallTarget callTarget) {
return new InternalMethod(sharedMethodInfo, name, declaringModule, visibility, false, proc, callTarget);
return new InternalMethod(sharedMethodInfo, name, declaringModule, visibility, false, proc, callTarget, null);
}

public InternalMethod(SharedMethodInfo sharedMethodInfo, String name, DynamicObject declaringModule,
Visibility visibility, CallTarget callTarget) {
this(sharedMethodInfo, name, declaringModule, visibility, false, null, callTarget);
this(sharedMethodInfo, name, declaringModule, visibility, false, null, callTarget, null);
}
public InternalMethod(SharedMethodInfo sharedMethodInfo, String name, DynamicObject declaringModule,
Visibility visibility, boolean undefined, DynamicObject proc, CallTarget callTarget) {
this(sharedMethodInfo, name, declaringModule, visibility, undefined, proc, callTarget, null);
}

private InternalMethod(SharedMethodInfo sharedMethodInfo, String name, DynamicObject declaringModule,
Visibility visibility, boolean undefined, DynamicObject proc, CallTarget callTarget) {
public InternalMethod(SharedMethodInfo sharedMethodInfo, String name, DynamicObject declaringModule,
Visibility visibility, boolean undefined, DynamicObject proc, CallTarget callTarget, DynamicObject capturedBlock) {
assert RubyGuards.isRubyModule(declaringModule);
this.sharedMethodInfo = sharedMethodInfo;
this.declaringModule = declaringModule;
@@ -59,6 +64,7 @@ private InternalMethod(SharedMethodInfo sharedMethodInfo, String name, DynamicOb
this.undefined = undefined;
this.proc = proc;
this.callTarget = callTarget;
this.capturedBlock = capturedBlock;
}

public SharedMethodInfo getSharedMethodInfo() {
@@ -91,28 +97,28 @@ public InternalMethod withDeclaringModule(DynamicObject newDeclaringModule) {
if (newDeclaringModule == declaringModule) {
return this;
} else {
return new InternalMethod(sharedMethodInfo, name, newDeclaringModule, visibility, undefined, proc, callTarget);
return new InternalMethod(sharedMethodInfo, name, newDeclaringModule, visibility, undefined, proc, callTarget, capturedBlock);
}
}

public InternalMethod withName(String newName) {
if (newName.equals(name)) {
return this;
} else {
return new InternalMethod(sharedMethodInfo, newName, declaringModule, visibility, undefined, proc, callTarget);
return new InternalMethod(sharedMethodInfo, newName, declaringModule, visibility, undefined, proc, callTarget, capturedBlock);
}
}

public InternalMethod withVisibility(Visibility newVisibility) {
if (newVisibility == visibility) {
return this;
} else {
return new InternalMethod(sharedMethodInfo, name, declaringModule, newVisibility, undefined, proc, callTarget);
return new InternalMethod(sharedMethodInfo, name, declaringModule, newVisibility, undefined, proc, callTarget, capturedBlock);
}
}

public InternalMethod undefined() {
return new InternalMethod(sharedMethodInfo, name, declaringModule, visibility, true, proc, callTarget);
return new InternalMethod(sharedMethodInfo, name, declaringModule, visibility, true, proc, callTarget, capturedBlock);
}

public boolean isVisibleTo(Node currentNode, DynamicObject callerClass) {
@@ -161,4 +167,7 @@ public Set<DynamicObject> getAdjacentObjects() {
return adjacent;
}

public DynamicObject getCapturedBlock() {
return capturedBlock;
}
}
19 changes: 11 additions & 8 deletions truffle/src/main/java/org/jruby/truffle/translator/BodyTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -797,7 +797,7 @@ public RubyNode visitCaseNode(org.jruby.ast.CaseNode node) {
return addNewlineIfNeeded(node, ret);
}

private RubyNode openModule(SourceSection sourceSection, RubyNode defineOrGetNode, String name, org.jruby.ast.Node bodyNode) {
private RubyNode openModule(SourceSection sourceSection, RubyNode defineOrGetNode, String name, org.jruby.ast.Node bodyNode, boolean sclass) {
LexicalScope newLexicalScope = environment.pushLexicalScope();
try {
final SharedMethodInfo sharedMethodInfo = new SharedMethodInfo(sourceSection, newLexicalScope, Arity.NO_ARGUMENTS, name, false, null, false, false, false);
@@ -807,7 +807,7 @@ private RubyNode openModule(SourceSection sourceSection, RubyNode defineOrGetNod

final BodyTranslator moduleTranslator = new BodyTranslator(currentNode, context, this, newEnvironment, source, false);

final MethodDefinitionNode definitionMethod = moduleTranslator.compileClassNode(sourceSection, name, bodyNode);
final MethodDefinitionNode definitionMethod = moduleTranslator.compileClassNode(sourceSection, name, bodyNode, sclass);

return new OpenModuleNode(context, sourceSection, defineOrGetNode, definitionMethod, newLexicalScope);
} finally {
@@ -824,7 +824,7 @@ private RubyNode openModule(SourceSection sourceSection, RubyNode defineOrGetNod
* newly allocated module or class.
* </p>
*/
private MethodDefinitionNode compileClassNode(SourceSection sourceSection, String name, org.jruby.ast.Node bodyNode) {
private MethodDefinitionNode compileClassNode(SourceSection sourceSection, String name, org.jruby.ast.Node bodyNode, boolean sclass) {
RubyNode body;

parentSourceSection.push(sourceSection);
@@ -840,12 +840,15 @@ private MethodDefinitionNode compileClassNode(SourceSection sourceSection, Strin

final RubyRootNode rootNode = new RubyRootNode(context, sourceSection, environment.getFrameDescriptor(), environment.getSharedMethodInfo(), body, environment.needsDeclarationFrame());

return new MethodDefinitionNode(
final MethodDefinitionNode definitionNode = new MethodDefinitionNode(
context,
sourceSection,
environment.getSharedMethodInfo().getName(),
environment.getSharedMethodInfo(),
Truffle.getRuntime().createCallTarget(rootNode));
Truffle.getRuntime().createCallTarget(rootNode),
sclass);

return definitionNode;
}

@Override
@@ -865,7 +868,7 @@ public RubyNode visitClassNode(org.jruby.ast.ClassNode node) {

final DefineOrGetClassNode defineOrGetClass = new DefineOrGetClassNode(context, sourceSection, name, lexicalParent, superClass);

final RubyNode ret = openModule(sourceSection, defineOrGetClass, name, node.getBodyNode());
final RubyNode ret = openModule(sourceSection, defineOrGetClass, name, node.getBodyNode(), false);
return addNewlineIfNeeded(node, ret);
}

@@ -1957,7 +1960,7 @@ public RubyNode visitModuleNode(org.jruby.ast.ModuleNode node) {

final DefineOrGetModuleNode defineModuleNode = new DefineOrGetModuleNode(context, sourceSection, name, lexicalParent);

final RubyNode ret = openModule(sourceSection, defineModuleNode, name, node.getBodyNode());
final RubyNode ret = openModule(sourceSection, defineModuleNode, name, node.getBodyNode(), false);
return addNewlineIfNeeded(node, ret);
}

@@ -2675,7 +2678,7 @@ public RubyNode visitSClassNode(org.jruby.ast.SClassNode node) {

final SingletonClassNode singletonClassNode = SingletonClassNodeGen.create(context, sourceSection, receiverNode);

final RubyNode ret = openModule(sourceSection, singletonClassNode, "(singleton-def)", node.getBodyNode());
final RubyNode ret = openModule(sourceSection, singletonClassNode, "(singleton-def)", node.getBodyNode(), true);
return addNewlineIfNeeded(node, ret);
}

2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/translator/MethodTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -212,7 +212,7 @@ public MethodDefinitionNode compileMethodNode(SourceSection sourceSection, Strin
context, body.getSourceSection(), environment.getFrameDescriptor(), environment.getSharedMethodInfo(), body, environment.needsDeclarationFrame());

final CallTarget callTarget = Truffle.getRuntime().createCallTarget(rootNode);
return new MethodDefinitionNode(context, sourceSection, methodName, environment.getSharedMethodInfo(), callTarget);
return new MethodDefinitionNode(context, sourceSection, methodName, environment.getSharedMethodInfo(), callTarget, false);
}

private void declareArguments(SourceSection sourceSection, String methodName, SharedMethodInfo sharedMethodInfo) {

4 comments on commit 71ae5d4

@eregon
Copy link
Member

@eregon eregon commented on 71ae5d4 Jan 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like too much adding stuff in InternalMethod that is not necessary for all methods.
Maybe we could handle sclass definitions without an additional method, by just going directly into it with the same translator (like say a while loop) and adapting the default definee and self? (or make it behave like singleton_class.class_eval { ... })
return might work without any special handling then :) (but other stuff might not "just work")

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm open to any other solution. I didn't know about the default definee stuff when I wrote this, so I can see the overlap.

Anything that passes the specs and doesn't damage performance is fine :)

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably wouldn't like my solution to the other spec - the instance_eval one - where I was planning to capture the default definee in the method in the same way, so I'll leave this def spec to you instead.

Sorry, something went wrong.

@eregon
Copy link
Member

@eregon eregon commented on 71ae5d4 Jan 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisseaton I think there is no other way for that def spec unfortunately, but I'll have a look.

Sorry, something went wrong.

Please sign in to comment.