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

[Truffle] Adding #to_int to Array#at paramters. #2685

Merged
merged 1 commit into from Mar 11, 2015

Conversation

bjfish
Copy link
Contributor

@bjfish bjfish commented Mar 11, 2015

I removed lowerFixnumParameter because it wasn’t handling double.

I’m not sure if the guard is needed?

@@ -446,10 +449,11 @@ public Object setIntegerFixnumRange(VirtualFrame frame, RubyArray array, RubyRan

}

@CoreMethod(names = "at", required = 1, lowerFixnumParameters = 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Safe to remove this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Per my comment on the PR, I think this should stay and a new specialization for double added.

@nirvdrum
Copy link
Contributor

We could have FixnumLowerNode truncate, but that's probably not safe in every case. I think you're going to need a new a @Specialization on double that does the correct conversion. I think for the most part, Ruby truncates and then uses the resulting Fixnum for the index.

nirvdrum added a commit that referenced this pull request Mar 11, 2015
[Truffle] Adding #to_int to Array#at paramters.
@nirvdrum nirvdrum merged commit 06be1c0 into jruby:master Mar 11, 2015
@chrisseaton chrisseaton added this to the truffle-dev milestone Mar 11, 2015
@bjfish bjfish deleted the truffle_array_at branch March 12, 2015 13:17
@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