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.

Sign up for GitHub

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

Jump to bottom

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

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

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

cdwijayarathna opened this issue May 9, 2015 · 4 comments
Labels
core JRuby 9000
Milestone
JRuby 9.0.0.0.rc1

Comments

@cdwijayarathna
Copy link
Contributor

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
Copy link
Member

headius 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
Copy link
Member

headius 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
Copy link
Contributor Author

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
Issue jruby#2924
8cd14f8
cdwijayarathna added a commit to cdwijayarathna/jruby that referenced this issue May 14, 2015
@cdwijayarathna
Issue jruby#2924
8ff6d2f
cdwijayarathna added a commit to cdwijayarathna/jruby that referenced this issue May 14, 2015
@cdwijayarathna
Issue jruby#2924
3862843
@cdwijayarathna cdwijayarathna mentioned this issue May 14, 2015
Closed
headius added a commit that referenced this issue May 19, 2015
@headius
Merge pull request #2939 from cdwijayarathna/checktype2
199f7bb 
Issue #2924
@headius
Copy link
Member

headius commented May 19, 2015

Fixed by #2930. Thanks, @cdwijayarathna!

@headius headius closed this as completed 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
Assignees
No one assigned
Labels
core JRuby 9000
Projects
None yet
Milestone
JRuby 9.0.0.0.rc1
Development

No branches or pull requests
2 participants
@headius @cdwijayarathna