-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
[Truffle] Correcting Array#inspect spec with inspect fixes #3997
Conversation
@@ -419,6 +419,22 @@ def self.compatible_encoding(a, b) | |||
enc | |||
end | |||
|
|||
# similar to rb_inspect | |||
def self.object_inspect(val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubinius::Type.object_*
are usually for the original version of these methods on Object/Kernel, like class
, etc.
Maybe just inspect
here is fine? (so we just remove the rb_
prefix)
Technically, it's OK since it's just a Java It would also avoid the wrapping/unwrapping which seems wrong (
It's not ideal because it means it's less easy to optimize and might throw jruby exceptions. |
@eregon I've refactored per your suggestions, thanks! |
@@ -3103,6 +3123,19 @@ protected static boolean hasRawBytes(DynamicObject string) { | |||
|
|||
} | |||
|
|||
@CoreMethod(names = "string_escape") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a primitive, like the node just below.
You can then invoke it with Truffle.invoke_primitive :string_escape, str
.
This avoids polluting String's namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eregon Made it a primitive in the latest commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using invoke_primitive
? I'll push a commit to do that.
cc @jruby/truffle |
This Array#inspect spec was failing due to issues with the current
String#inspect
and lack ofrb_inspect
andrb_str_escape
. I had to get creative/experimental to resolve all these issues.Any issues with the following?
Are there any issue with using Truffle::CExt nodes like this in our ruby core code?
Are there any issues using the
RubyString
implementations?