Difference between Ruby's Check_Type() and implementation in TruffleRuby

[kwilczynski]

Hi @eregon, sorry for late reply!

[...]

The nil type vs NilClass is probably something that JRuby and TruffleRuby could fix, if desired. What do you think @eregon?

Yes, that's worth reporting. If you can tell which C API function does that it would be helpful, then we should be able to easily write a test for it. nil/true/false are typically treated specially in error messages, so we might have missed this specific case.

OK. I will have a look and report back. Thank you!
[...]

I was able to derive a small piece of code that can reproduce what I was talking about:

#if defined(__cplusplus)
extern "C" {
#endif

#include <ruby.h>

void
Init_test(void)
{
	printf("%s\n", rb_class2name(rb_cNilClass));
	printf("%s\n", rb_obj_classname(Qnil));

	printf("%s\n", rb_class2name(rb_cString));
	printf("%s\n", rb_obj_classname(rb_str_new2("")));

	/* Check_Type(Qnil, T_STRING); */
	/* Check_Type(INT2NUM(0), T_STRING); */
	rb_check_type(Qnil, T_STRING);
}

#if defined(__cplusplus)
}
#endif

# frozen_string_literal: true

require 'mkmf'

dir_config('test')

create_header
create_makefile('test')

Running it against Ruby and TruffleRuby:

• Ruby:

   kwilczynski@rocinante  Ruby/test  $ rbenv local 3.0.0
   kwilczynski@rocinante  Ruby/test  $ ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
   kwilczynski@rocinante  Ruby/test  $ ruby extconf.rb
creating extconf.h
creating Makefile
   kwilczynski@rocinante  Ruby/test  $ make
compiling test.c
linking shared-object test/test.bundle
   kwilczynski@rocinante  Ruby/test  $ RUBYLIB=. ruby -rtest -e ''
NilClass
NilClass
String
String
<internal:/Users/kwilczynski/.rbenv/versions/3.0.0/lib/ruby/3.0.0/rubygems/core_ext/kernel_require.rb>:85:in `require': wrong argument type nil (expected String) (TypeError)
	from <internal:/Users/kwilczynski/.rbenv/versions/3.0.0/lib/ruby/3.0.0/rubygems/core_ext/kernel_require.rb>:85:in `require'

• TruffleRuby:

   kwilczynski@rocinante  Ruby/test  $ rbenv local truffleruby-21.0.0
   kwilczynski@rocinante  Ruby/test  $ ruby -v
truffleruby 21.0.0, like ruby 2.7.2, GraalVM CE Native [x86_64-darwin]
   kwilczynski@rocinante  Ruby/test  $ ruby extconf.rb
creating extconf.h
creating Makefile
   kwilczynski@rocinante  Ruby/test  $ make
compiling test.c
linking shared-object test/test.bundle
   kwilczynski@rocinante  Ruby/test  $ RUBYLIB=. ruby -rtest -e ''
NilClass
NilClass
String
String
/Users/kwilczynski/.rbenv/versions/truffleruby-21.0.0/lib/truffle/truffle/cext.rb:208:in `rb_check_type': wrong argument type NilClass (expected 5) (TypeError)
	from object.c:35:in `rb_check_type'
	from /Users/kwilczynski/Development/Projects/Personal/Ruby/test/test.c:16:in `Init_test'

The function I am using is Check_Type() from include/ruby/internal/value_type.h#L329-L352:

RBIMPL_ATTR_ARTIFICIAL()
static inline void
Check_Type(VALUE v, enum ruby_value_type t)
{
    if (RB_UNLIKELY(! RB_TYPE_P(v, t))) {
        goto slowpath;
    }
    else if (t != RUBY_T_DATA) {
        goto fastpath;
    }
    else if (rbimpl_rtypeddata_p(v)) {
        /* The intention itself is not necessarily clear to me, but at least it
         * is  intentional   to  rule   out  typed   data  here.    See  commit
         * a7c32bf81d3391cfb78cfda278f469717d0fb794. */
        goto slowpath;
    }
    else {
        goto fastpath;
    }

  fastpath:
    return;

  slowpath: /* <- :TODO: mark this label as cold. */
    rb_check_type(v, t);
}

Which in Ruby is trying to do some internal optimisations to make the resolution a bit faster, but then it might end up calling the rb_check_type() from error.c#L984-L997:

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

    if (x == Qundef) {
	rb_bug(UNDEF_LEAKED);
    }

    xt = TYPE(x);
    if (xt != t || (xt == T_DATA && RTYPEDDATA_P(x))) {
	unexpected_type(x, xt, t);
    }
}

TruffleRuby also provides Check_Type(), albeit it's a macro (see: lib/cext/include/ruby/ruby.h#L664) that wraps rb_check_type(), as per:

#define Check_Type(v,t) rb_check_type((VALUE)(v),(t))

And the rb_check_type() is then defined (see: src/main/c/cext/object.c#L43-L45) as follows:

void rb_check_type(VALUE value, int type) {
  polyglot_invoke(RUBY_CEXT, "rb_check_type", rb_tr_unwrap(value), type);
}

I then found a bit of documentation on what might be happening internally:

cexts.md#c-runtime-implementation-details.

Should I go deeper into finding where things diverge, or would it be enough for you to decide whether this is something that you would consider something to potentially fix? Let me know. :)

Krzysztof

Originally posted by @kwilczynski in kwilczynski/ruby-magic#10 (comment)

[eregon]

Thanks for the great bug report.
We should fix the error message for rb_check_type, so that it matches CRuby.

[kwilczynski]

Hello,

Just to add - prior to Ruby 3.0, the Check_Type() has also been a macro wrapping a number of functions - somewhat less sophisticated than what recent versions of Ruby offers, as per:

#define Check_Type(v, t) \
    (!RB_TYPE_P((VALUE)(v), (t)) || \
     ((t) == RUBY_T_DATA && RTYPEDDATA_P(v)) ? \
     rb_unexpected_type((VALUE)(v), (t)) : (void)0)

N.B. This has no bearing on the message that Ruby would emit, whether it would be 2.7 or 3.0.

Krzysztof

[eregon]

@aardvark179 Could you look at this one? Probably we need some kind of mapping from T_STRING to String and for all other T_*.

[kwilczynski]

Hi @eregon and @aardvark179,

[...]
Probably we need some kind of mapping from T_STRING to String and for all other T_*.

Not sure if this helps, but I added a small helper to my Ruby Gem:

• kwilczynski/ruby-magic@e18179d
• kwilczynski/ruby-magic@7a05a0c

Not sure if this was the best way, but does work and normalizes the type checking between Ruby and TruffleRuby a little bit, especially when it goes to error reporting.

Krzysztof

[eregon]

@kwilczynski Fixed in ea82ff0 by @aardvark179 and myself.

I would recommend switching to truffleruby-head (nightly builds) in CI and remove the workarounds: kwilczynski/ruby-magic@e18179d & kwilczynski/ruby-magic@7a05a0c.
In general we want to avoid TruffleRuby-specific code in gems unless strictly needed, and it also makes it harder to debug if something goes wrong.
TruffleRuby users are typically fine to use the nightly builds and not releases, given the releases are not that frequent and there is always a lot of recent changes worth trying.

Or you can wait for the 20.2.0 release if you prefer, but that will be in a while, and I'm concerned we would forget about those workarounds: https://www.graalvm.org/release-notes/version-roadmap/

I think just skipping the couple failing tests on TruffleRuby <= 20.1.0 is also completely fine.

[kwilczynski]

Hi @eregon,

Thank you for update! Also, apologies for making you and @aardvark179 do more work! I appreciate to fix, though. :)

Re: truffleruby-head - that I wasn't sure about, good to know. I will move the CI to use this nightly too.

Krzysztof