Skip to content

Commit

Permalink
[Truffle] Fix new TCK.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Sep 12, 2015
1 parent 26985d0 commit 053458a
Showing 4 changed files with 44 additions and 9 deletions.
37 changes: 34 additions & 3 deletions truffle/src/main/java/org/jruby/truffle/nodes/interop/InteropNode.java
View file
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@
*/
package org.jruby.truffle.nodes.interop;

import com.oracle.truffle.api.CallTarget;
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.Truffle;
import com.oracle.truffle.api.dsl.Cached;
@@ -129,12 +130,33 @@ public ExecuteMethodNode(RubyContext context,
callNode = Truffle.getRuntime().createIndirectCallNode();
}

@Specialization(guards = {"isRubyProc(proc)", "proc == cachedProc"})
protected Object doCallProc(VirtualFrame frame, DynamicObject proc,
@Cached("proc") DynamicObject cachedProc,
@Cached("create(getCallTarget(cachedProc))") DirectCallNode callNode) {
final List<Object> faArgs = ForeignAccess.getArguments(frame);
Object[] args = faArgs.toArray();
return callNode.call(frame, RubyArguments.pack(Layouts.PROC.getMethod(cachedProc), Layouts.PROC.getDeclarationFrame(cachedProc), Layouts.PROC.getSelf(cachedProc), null, args));
}

@Specialization(guards = "isRubyProc(proc)")
protected Object doCallProc(VirtualFrame frame, DynamicObject proc) {
final List<Object> faArgs = ForeignAccess.getArguments(frame);
Object[] args = faArgs.toArray();
return callNode.call(frame, Layouts.PROC.getCallTargetForType(proc), RubyArguments.pack(
Layouts.PROC.getMethod(proc),
Layouts.PROC.getDeclarationFrame(proc),
Layouts.PROC.getSelf(proc),
null,
args));
}

@Specialization(guards = {"isRubyMethod(method)", "method == cachedMethod"})
protected Object doCall(VirtualFrame frame, DynamicObject method,
@Cached("method") DynamicObject cachedMethod,
@Cached("getMethod(cachedMethod)") InternalMethod internalMethod,
@Cached("create(getMethod(cachedMethod).getCallTarget())") DirectCallNode callNode) {
final List<Object> faArgs = ForeignAccess.getArguments(frame);
final List<Object> faArgs = ForeignAccess.getArguments(frame);
// skip first argument; it's the receiver but a RubyMethod knows its receiver
Object[] args = faArgs.subList(1, faArgs.size()).toArray();
return callNode.call(frame, RubyArguments.pack(internalMethod, internalMethod.getDeclarationFrame(), Layouts.METHOD.getReceiver(cachedMethod), null, args));
@@ -143,7 +165,7 @@ protected Object doCall(VirtualFrame frame, DynamicObject method,
@Specialization(guards = "isRubyMethod(method)")
protected Object doCall(VirtualFrame frame, DynamicObject method) {
final InternalMethod internalMethod = Layouts.METHOD.getMethod(method);
final List<Object> faArgs = ForeignAccess.getArguments(frame);
final List<Object> faArgs = ForeignAccess.getArguments(frame);
// skip first argument; it's the receiver but a RubyMethod knows its receiver
Object[] args = faArgs.subList(1, faArgs.size()).toArray();
return callNode.call(frame, internalMethod.getCallTarget(), RubyArguments.pack(
@@ -154,6 +176,14 @@ protected Object doCall(VirtualFrame frame, DynamicObject method) {
args));
}

protected InternalMethod getMethodFromProc(DynamicObject proc) {
return Layouts.PROC.getMethod(proc);
}

protected CallTarget getCallTarget(DynamicObject proc) {
return Layouts.PROC.getCallTargetForType(proc);
}

protected InternalMethod getMethod(DynamicObject method) {
return Layouts.METHOD.getMethod(method);
}
@@ -167,7 +197,8 @@ public InteropIsExecutable(RubyContext context, SourceSection sourceSection) {

@Override
public Object execute(VirtualFrame frame) {
return RubyGuards.isRubyMethod(ForeignAccess.getReceiver(frame));
final Object receiver = ForeignAccess.getReceiver(frame);
return RubyGuards.isRubyMethod(receiver) || RubyGuards.isRubyProc(receiver);
}

}
2 changes: 2 additions & 0 deletions truffle/src/main/java/org/jruby/truffle/runtime/RubyObjectType.java
View file
Original file line number Diff line number Diff line change
@@ -46,6 +46,8 @@ public String toString(DynamicObject object) {
public ForeignAccess getForeignAccessFactory() {
if (Layouts.METHOD.isMethod(this)) {
return RubyMethodForeignAccessFactory.create(getContext());
} else if (Layouts.PROC.isProc(this)) {
return RubyMethodForeignAccessFactory.create(getContext());
} else if (Layouts.ARRAY.isArray(this)) {
return ArrayForeignAccessFactory.create(getContext());
} else if (Layouts.HASH.isHash(this)) {
2 changes: 2 additions & 0 deletions truffle/src/main/java/org/jruby/truffle/runtime/layouts/ProcLayout.java
View file
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
import com.oracle.truffle.api.frame.MaterializedFrame;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.object.DynamicObjectFactory;
import com.oracle.truffle.api.object.ObjectType;
import org.jruby.truffle.nodes.core.ProcNodes;
import org.jruby.truffle.om.dsl.api.Layout;
import org.jruby.truffle.om.dsl.api.Nullable;
@@ -43,6 +44,7 @@ DynamicObject createProc(
Object self,
@Nullable DynamicObject block);

boolean isProc(ObjectType objectType);
boolean isProc(DynamicObject object);
boolean isProc(Object object);

12 changes: 6 additions & 6 deletions truffle/src/test/java/org/jruby/truffle/tck/RubyTckTest.java
View file
Original file line number Diff line number Diff line change
@@ -46,12 +46,12 @@ protected TruffleVM prepareVM() throws Exception {
+ " Truffle::Interop.execute(f, 18, 32) + 10\n"
+ "end\n"
+ "def compound_object\n"
+ " Module.new do\n"
+ " def self.fourtyTwo; 42; end\n"
+ " def self.plus(a, b); a + b; end\n"
+ " def self.returnsNull; nil; end\n"
+ " def self.returnsThis; self; end\n"
+ " end\n"
+ " object = Object.new\n"
+ " def object.fourtyTwo; 42; end\n"
+ " def object.plus; -> (a, b) { a + b }; end\n"
+ " def object.returnsNull; nil; end\n"
+ " def object.returnsThis; self; end\n"
+ " object\n"
+ "end\n"

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Sep 12, 2015

Author Contributor

@jtulach this is what I had to do in the end to get the TCK compound object test passing. I think the Java interop system specifically calls read and then execute, so for plus I'm having to explicitly return a function, like you would in JS. We're going to have to think about how to make this work well for all languages, as the current system seems very much like hard-coded JS semantics.
+ "Truffle::Interop.export(\"sum_ints\", method(:sum))\n"
+ "Truffle::Interop.export(\"fourty_two\", method(:fourty_two))\n"

8 comments on commit 053458a

@jtulach
Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Plus returns a lambda. If I send INVOKE - will it be better?

@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 think that would work - but I worry we're just trying random things until they work. Maybe we should have a document with what objects look like in different languages, and how they should respond to different messages, and then add tests for that to the TCK. I imagine in writing that document we would discover that some things are contradictory.

@jtulach
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I am trying to document how interop messages should behave. The simple ones 1st: https://lafo.ssw.uni-linz.ac.at/hg/truffle/rev/5857f5ee9486 - is there anything (Ruby specific) you want to add to the text? Btw. the Javadoc is at http://lafo.ssw.uni-linz.ac.at/javadoc/truffle/latest/com/oracle/truffle/api/interop/Message.html

@jtulach
Copy link
Contributor

Choose a reason for hiding this comment

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

I did my best at https://lafo.ssw.uni-linz.ac.at/hg/truffle/rev/cc195dd45121, anything to change from Ruby point of view?

@eregon
Copy link
Member

@eregon eregon commented on 053458a Sep 15, 2015

Choose a reason for hiding this comment

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

Looks great to me. I have a question about why we need to pass the number of arguments but that's unrelated.

@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.

The messages all look well documented. I think what we need to document now is what the different languages send for different syntax, and what things like the Java interop will send. On the Ruby side the protocol is currently very fragile. I'll start documenting how Ruby responds to messages and the messages it sends.

@jtulach
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to add some examples with languages I knew (e.g. Java, C, possibly JavaScript) into the Truffle documentation. Do you want to have your own document (pros: you control it, cons: nobody finds it) or would you consider to add the sample Ruby code into the Javadoc of Truffle?

NetBeans has a concept of non-normative sections in Javadoc (like createAndOpen and addRecursiveListener). It tries to explain how some API concept are really implemented in parts of the system that don't have their own API/documentation, but are essential for understanding how the API behaves.

Maybe it would be beneficial to include the Ruby samples next to the Message elements (pros: instantly ready for API readers, cons: hard to keep them in sync)!?

Sorry, something went wrong.

@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.

I'll certainly start to write them - we can tweak where they live at any point. They probably belong in Truffle documentation because people are going to want to know how key languages will respond to them, and if we have a range of languages they can also use that to determine what they should do for their own languages.

Sorry, something went wrong.

Please sign in to comment.