Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopting to Redesigned Truffle Inter-operability API #3017

Merged
merged 3 commits into from
Jun 13, 2015
Merged

Adopting to Redesigned Truffle Inter-operability API #3017

merged 3 commits into from
Jun 13, 2015

Conversation

jtulach
Copy link
Contributor

@jtulach jtulach commented Jun 4, 2015

I am in process of redesigning com.oracle.truffle.api.interop API. Once that gets into development version of Truffle, JRuby+Truffle will need to be adopted. This pull request shows what changes I had to do make things running.


public WritePropertyNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
this.node = ForeignObjectAccessNode.getAccess(Write.create(Receiver.create(), Argument.create(), Argument.create()));
this.node = ForeignAccess.node(ForeignAccess.msgWrite());
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these node methods factories or something? The name is strange - being a noun rather than a verb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Christian Humer had similar comment. He wanted to rename node to createNode and msgXYZ to something else as well. The msgXYZ methods are however more like singleton accessors than factories.

Copy link
Member

Choose a reason for hiding this comment

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

I think as well createNode would be clearer if it is a factory method, .node could be like a getter, or possibly implies some caching (does it?).
Would an enum or constants make sense for the msgXYZ if they are singleton instances?

@chrisseaton
Copy link
Contributor

This all looks fine. Do you want this merged now, or later?

@jtulach
Copy link
Contributor Author

jtulach commented Jun 5, 2015

My plan is to integrate the API changes on Monday. Then you'll need this PR.

@chrisseaton chrisseaton merged commit 1dc51ef into jruby:truffle-head Jun 13, 2015
@chrisseaton chrisseaton added this to the truffle-dev milestone Jun 13, 2015
@chrisseaton chrisseaton self-assigned this Jun 13, 2015
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants