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

Issue in org.jruby.util.TypeConverter CheckType #2924

Closed
cdwijayarathna opened this Issue May 9, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@cdwijayarathna
Copy link
Contributor

commented May 9, 2015

I tried to use above mentioned method to check it a IRubyObject is a RubyHash or not,
Following is how I used it.
TypeConverter.checkType(context, opts, context.runtime.getHash());
When I give a Hash as opts, my irb interface I get following output.

TypeError: wrong argument type Hash (expected Hash)

I'm sure that here it should be "Wrong arguement type ",
because in https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/TypeConverter.java#L300 it has used the reference class to (RubyModule t) to identify the type of the object ('x').So I believe #L300 should be x.someMethod(), not t.someMethod().

Other issue is, when I pass a Hash, to this it gives this TypeError. I think the issue is in https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/TypeConverter.java#L299,
x.getMetaClass().getNativeClassIndex() refers to "CLASS" while t.getClassIndex() refers to "HASH".
I tried with other types as well and I didn't get true when types match at any time.

@headius

This comment has been minimized.

Copy link
Member

commented May 10, 2015

Reproduction in Ruby:

runtime = JRuby.runtime
context = runtime.current_context
opts = {foo: 'bar'}
org.jruby.util.TypeConverter.check_type(context, opts, runtime.hash)
@headius

This comment has been minimized.

Copy link
Member

commented May 10, 2015

Chamila: I think you're probably right. Only one place in JRuby is using this method, so it must be stale or perhaps it never worked properly at all.

You should compare our version with MRI's version and make them match in a PR.

@cdwijayarathna

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2015

MRI implementation

void
rb_check_type(VALUE x, int t)
{
    int xt;

    if (x == Qundef) {
    rb_bug("undef leaked to the Ruby space");
    }

    xt = TYPE(x);
    if (xt != t || (xt == T_DATA && RTYPEDDATA_P(x))) {
    const char *tname = rb_builtin_type_name(t);
    if (tname) {
        rb_raise(rb_eTypeError, "wrong argument type %s (expected %s)",
             builtin_class_name(x), tname);
    }
    if (xt > T_MASK && xt <= 0x3f) {
        rb_fatal("unknown type 0x%x (0x%x given, probably comes from extension library for ruby 1.8)", t, xt);
    }
    rb_bug("unknown type 0x%x (0x%x given)", t, xt);
    }
}

cdwijayarathna added a commit to cdwijayarathna/jruby that referenced this issue May 11, 2015

cdwijayarathna added a commit to cdwijayarathna/jruby that referenced this issue May 14, 2015

cdwijayarathna added a commit to cdwijayarathna/jruby that referenced this issue May 14, 2015

headius added a commit that referenced this issue May 19, 2015

@headius

This comment has been minimized.

Copy link
Member

commented May 19, 2015

Fixed by #2930. Thanks, @cdwijayarathna!

@headius headius closed this May 19, 2015

@headius headius added this to the JRuby 9.0.0.0.rc1 milestone May 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.