[Truffle] Allow Rubinius primitives to be written entirely in Ruby #3076
Conversation
| @@ -31,6 +30,7 @@ def self.module_mirror(obj) | |||
| end | |||
| end | |||
| end | |||
| Truffle::Primitive.install_rubinius_primitive self, :module_mirror | |||
There was a problem hiding this comment.
I'm not sure how to get self in a @CoreMethod anymore - onSingleton and isModuleFunction both set needsSelf to false.
There was a problem hiding this comment.
In this particular case, it's intended to not be obvious, since it would be likely wrong with a module function (defined on both instance and class level) as self could be either a module or the surrounding self.
If it is just a singleton/class method, then it might be right to want it and currently there is constuctor for this (which is literally onSingleton + needsSelf), which was appropriately named when I looked at current use-cases.
There was a problem hiding this comment.
Yes I thought that self was the module when it isn't. I think I will use some magic to get the self of the caller, since this slow path stuff.
| new LiteralNode(context, sourceSection, callConstructor.getModule()), | ||
| null, | ||
| false, | ||
| arguments.toArray(new RubyNode[arguments.size()])); |
There was a problem hiding this comment.
Can we share this with above so the code is not duplicated?
There was a problem hiding this comment.
It's only the arguments array that can be shared. I didn't do it originally as the body of the loop is a bit different, but I can probably do a second pass to handle that.
|
Sounds like a nice addition. Truffle::Primitive.install_rubinius_primitive :vm_object_class, Kernel.method(:class)Which would solve the currently not very useful primitives just delegating to something else and possible monkey-patching issues. |
|
Yes that's a good idea. The main motivation for this was to start writing some primitives using Java interop. |
|
This looks good. However, I am concerned about the ability to find these primitive methods. While having a node that serves only to delegate to a RubyCallNode seems superfluous at face value, it does have the benefit of being consistent with primitives written in Java. I.e., they can all be found in an expected location. If we want to go this route, I propose introducing a new directory for implemented mirror methods. I would maybe even take it a step further and have the translator look at the source section to ensure Truffle::Primitive.install_rubinius_primitive can only be used from this directory, throwing an error otherwise. That might be a bit heavy-handed (particularly for a lint tool), but it would help keep us honest. |
|
A new directory for mirror methods? Do you mean a directory for primitive methods? |
|
Sorry, yes, for primitives. The mirror stuff I think could be tidied up a bit too, but that's a separate issue. |
|
I agree, but maybe a file is enough? It should be loaded just before alpha.rb I think. |
|
A single file is fine with me. I just don't want that to become unwieldly. Ruby is much more concise than Java for this though, so it may not be a warranted concern. |
|
I'll do the change to make it take a Ruby Method. |
…th Truffle::Primitive.install_rubinius_primitive
eregon
force-pushed
the
truffle-ruby-primitives
branch
from
4be9644 to
364d745
Compare
* Receiver + method name in one argument.
|
👍 looks good to me |
* Since we can add primitives at runtime.
* Extract logic from BodyTranslator into RubiniusPrimitiveConstructor implementations.
[Truffle] Allow Rubinius primitives to be written entirely in Ruby
|
I merged with a few changes to accept (bound) Method. I tried as well with UnboundMethod, but it does not fit for |
@eregon @nirvdrum @pitr-ch please review and sign off when you are done.