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] Rubinius::Type.check_convert_type #3992

Closed
bjfish opened this issue Jul 2, 2016 · 5 comments
Closed

[Truffle] Rubinius::Type.check_convert_type #3992

bjfish opened this issue Jul 2, 2016 · 5 comments
Assignees
Milestone

Comments

@bjfish
Copy link
Contributor

bjfish commented Jul 2, 2016

This Rubinius::Type.check_convert_type comment says that it corresponds to rb_check_convert_type but it appears that it doesn't handle respond_to_missing? or missing_method methods.

Correcting this should fix the array/flatten_spec.rb flatten performs respond_to? and method_missing-aware checks when coercing elements to array.

Rubinius::Type.coerce_to should probably be reviewed too.

@bjfish bjfish added the truffle label Jul 2, 2016
@eregon
Copy link
Member

eregon commented Jul 2, 2016

This should be handled by the default respond_to?, no?
check_convert_type uses vm_object_respond_to, which uses Kernel#respond_to?, so I am not sure what's going wrong here.

@eregon
Copy link
Member

eregon commented Jul 2, 2016

The issue seems to be there is no equivalent of rb_check_funcall at the Ruby level.

@bjfish
Copy link
Contributor Author

bjfish commented Jul 2, 2016

@eregon Could there be an issue with vm_object_respond_to ? The following example:

class Test
  def respond_to?(method_name, include_private = false)
    puts "IN RESPOND TO"
    true
  end
end
test = Test.new
puts "test.respond_to? :to_ary - #{test.respond_to? :to_ary}"
puts "Rubinius::Type.object_respond_to? test, :to_ary, true - #{Rubinius::Type.object_respond_to? test, :to_ary, true}"

Outputs:

IN RESPOND TO
test.respond_to? :to_ary - true
Rubinius::Type.object_respond_to? test, :to_ary, true - false

@eregon
Copy link
Member

eregon commented Jul 2, 2016

@bjfish Yes, vm_object_respond_to is bound to BasicObject#respond_to?, so indeed we should probably try calling respond_to? dynamically. I tried a few things but gave up for now since it seems there are many differences here with MRI and it's quite hard to follow the code.

@bjfish
Copy link
Contributor Author

bjfish commented Sep 16, 2016

This was fixed by #4146

@bjfish bjfish closed this as completed Sep 16, 2016
@enebo enebo modified the milestone: truffle-dev Nov 9, 2016
@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

No branches or pull requests

3 participants