[GR-17457] Fixes for problems found in oj gem CI.

PullRequest: truffleruby/3453

Author: aardvark179

CHANGELOG.md

@@ -29,6 +29,10 @@ Compatibility:
 * Fix `Kernel#sprintf` formatting for `%c` when used non-ASCII encoding (#2369, @andrykonchin).
 * Fix `Kernel#sprintf` argument casting for `%c` (@andrykonchin).
 * Implement the `rb_enc_strlen` function for use by native extensions (@nirvdrum).
+* Match tag values used by `rb_protect` and `rb_jump_tag` for the `tk` gem (#2556, @aardvark179).
+* Implement `rb_eval_cmd_kw` to support the `tk` gem (#2556, @aardvark179).
+* Fix `rb_class2name` to call `inspect` on anonymous classes like in CRuby (#2701, @aardvark179).
+* Implement `rb_ivar_foreach` to iterate over instance and class variables like in CRuby (#2701, @aardvark179).
 
 Performance:
 

lib/cext/ABI_version.txt

@@ -1 +1 @@
-9
+10

lib/truffle/truffle/cext.rb

@@ -484,6 +484,34 @@ def rb_f_global_variables
     Kernel.global_variables
   end
 
+  def rb_ivar_foreach(object, func, arg)
+    keys_and_vals = []
+    if Module === object
+      keys_and_vals << :__classpath__
+      keys_and_vals << object.name
+
+      object.class_variables.each do |key|
+        keys_and_vals << key
+        keys_and_vals << object.class_variable_get(key)
+      end
+    end
+    object.instance_variables.each do |key|
+      keys_and_vals << key
+      keys_and_vals << object.instance_variable_get(key)
+    end
+
+    keys_and_vals.each_slice(2) do |key, val|
+      st_result = Truffle::Interop.execute_without_conversion(
+        func, Primitive.cext_sym2id(key), Primitive.cext_wrap(val), arg)
+
+      case st_result
+      when ST_CONTINUE
+      when ST_STOP then break
+      else raise ArgumentError, "Unknown 'func' return value: #{st_result}"
+      end
+    end
+  end
+
   def rb_obj_instance_variables(object)
     object.instance_variables
   end
@@ -1610,14 +1638,17 @@ def warning?
   end
 
   def rb_time_nano_new(sec, nsec)
-    Time.at sec, Rational(nsec, 1000)
+    Time.at sec, nsec, :nanosecond
   end
 
   def rb_time_timespec_new(sec, nsec, offset, is_utc, is_local)
-    time = rb_time_nano_new(sec, nsec)
-    return time if is_local
-    return time.getgm if is_utc
-    time.getlocal(offset)
+    if is_local
+      Time.at(sec, nsec, :nanosecond)
+    elsif is_utc
+      Time.at(sec, nsec, :nanosecond, in: 0).getgm
+    else
+      Time.at(sec, nsec, :nanosecond, in: offset)
+    end
   end
 
   def rb_time_num_new(timev, off)
@@ -1727,7 +1758,7 @@ def rb_class_inherited_p(ruby_module, object)
   def rb_get_special_vars
     vars = Primitive.cext_special_variables_from_stack
     unless vars
-      vars = Truffle::ThreadOperations.ruby_caller_special_variables([Truffle::CExt, Truffle::Interop.singleton_class])
+      vars = Truffle::ThreadOperations.ruby_caller_special_variables([Truffle::CExt, Truffle::CExt.singleton_class, Truffle::Interop.singleton_class])
     end
     vars
   end
@@ -1917,4 +1948,15 @@ def rb_exception_set_message(e, mesg)
   def rb_global_variable(obj)
     GLOBALLY_PRESERVED_VALUES << obj
   end
+
+  GC_REGISTERED_ADDRESSES = {}
+  def rb_gc_register_address(address, obj)
+    Truffle::Interop.to_native(address) unless Truffle::Interop.pointer?(address)
+    GC_REGISTERED_ADDRESSES[address] = obj
+  end
+
+  def rb_gc_unregister_address(address)
+    Truffle::Interop.to_native(address) unless Truffle::Interop.pointer?(address)
+    GC_REGISTERED_ADDRESSES.delete(address)
+  end
 end

spec/ruby/optional/capi/class_spec.rb

@@ -204,6 +204,10 @@
     it "returns a string for an anonymous class" do
       @s.rb_class2name(Class.new).should be_kind_of(String)
     end
+
+    it "returns a string beginning with # for an anonymous class" do
+      @s.rb_class2name(Struct.new(:x, :y).new(1, 2).class).should.start_with?('#')
+    end
   end
 
   describe "rb_class_path" do

spec/ruby/optional/capi/ext/object_spec.c

@@ -393,6 +393,18 @@ static VALUE object_spec_rb_class_inherited_p(VALUE self, VALUE mod, VALUE arg)
   return rb_class_inherited_p(mod, arg);
 }
 
+static int foreach_f(ID key, VALUE val, VALUE ary) {
+  rb_ary_push(ary, ID2SYM(key));
+  rb_ary_push(ary, val);
+  return ST_CONTINUE;
+}
+
+static VALUE object_spec_rb_ivar_foreach(VALUE self, VALUE obj) {
+  VALUE ary = rb_ary_new();
+  rb_ivar_foreach(obj, foreach_f, ary);
+  return ary;
+}
+
 static VALUE speced_allocator(VALUE klass) {
   VALUE flags = 0;
   VALUE instance;
@@ -508,6 +520,7 @@ void Init_object_spec(void) {
   rb_define_method(cls, "speced_allocator?", speced_allocator_p, 1);
   rb_define_method(cls, "custom_alloc_func?", custom_alloc_func_p, 1);
   rb_define_method(cls, "not_implemented_method", rb_f_notimplement, -1);
+  rb_define_method(cls, "rb_ivar_foreach", object_spec_rb_ivar_foreach, 1);
 }
 
 #ifdef __cplusplus

spec/ruby/optional/capi/fixtures/object.rb

@@ -0,0 +1,29 @@
+class CApiObjectSpecs
+  class IVars
+    def initialize
+      @a = 3
+      @b = 7
+      @c = 4
+    end
+
+    def self.set_class_variables
+      @@foo = :a
+      @@bar = :b
+      @@baz = :c
+    end
+  end
+
+  module MVars
+    @@mvar = :foo
+    @@mvar2 = :bar
+
+    @ivar = :baz
+  end
+
+  module CVars
+    @@cvar = :foo
+    @@cvar2 = :bar
+
+    @ivar = :baz
+  end
+end

spec/ruby/optional/capi/object_spec.rb

@@ -1,4 +1,5 @@
 require_relative 'spec_helper'
+require_relative 'fixtures/object'
 
 load_extension("object")
 
@@ -983,5 +984,24 @@ def reach
         @o.speced_allocator?(parent).should == true
       end
     end
+
+    describe "rb_ivar_foreach" do
+      it "calls the callback function for each instance variable on an object" do
+        o = CApiObjectSpecs::IVars.new
+        ary = @o.rb_ivar_foreach(o)
+        ary.should == [:@a, 3, :@b, 7, :@c, 4]
+      end
+
+      it "calls the callback function for each cvar and ivar on a class" do
+        ary = @o.rb_ivar_foreach(CApiObjectSpecs::CVars)
+        ary.should == [:__classpath__, 'CApiObjectSpecs::CVars', :@@cvar, :foo, :@@cvar2, :bar, :@ivar, :baz]
+      end
+
+      it "calls the callback function for each cvar and ivar on a module" do
+        ary = @o.rb_ivar_foreach(CApiObjectSpecs::MVars)
+        ary.should == [:__classpath__, 'CApiObjectSpecs::MVars', :@@mvar, :foo, :@@mvar2, :bar, :@ivar, :baz]
+      end
+
+    end
   end
 end

src/main/c/cext/class.c

@@ -71,7 +71,7 @@ VALUE rb_class_name(VALUE ruby_class) {
   VALUE name = RUBY_INVOKE(ruby_class, "name");
 
   if (NIL_P(name)) {
-    return rb_class_name(rb_obj_class(ruby_class));
+    return RUBY_INVOKE(ruby_class, "inspect");
   } else {
     return name;
   }

src/main/c/cext/gc.c

@@ -13,10 +13,14 @@
 // GC, rb_gc_*
 
 void rb_gc_register_address(VALUE *address) {
+  /* TODO: This should guard the address of the pointer, not the
+     object pointed to, but we haven't yet found a good way to implement
+     that, or a real world use case where it is required. */
+  polyglot_invoke(RUBY_CEXT, "rb_gc_register_address", address, rb_tr_unwrap(*address));
 }
 
 void rb_gc_unregister_address(VALUE *address) {
-  // VALUE is only ever in managed memory. So, it is already garbage collected.
+  polyglot_invoke(RUBY_CEXT, "rb_gc_unregister_address", address);
 }
 
 void rb_gc_mark(VALUE ptr) {

src/main/c/cext/ivar.c

@@ -50,7 +50,7 @@ VALUE rb_ivar_lookup(VALUE object, const char *name, VALUE default_value) {
 
 // Needed to gem install oj
 void rb_ivar_foreach(VALUE obj, int (*func)(ANYARGS), st_data_t arg) {
-  rb_tr_error("rb_ivar_foreach not implemented");
+  polyglot_invoke(RUBY_CEXT, "rb_ivar_foreach", rb_tr_unwrap(obj), func, (void*)arg);
 }
 
 VALUE rb_attr_get(VALUE object, ID name) {