[GR-18163] Fix arity of rb_define_method()-defined methods and add specs (#2662)

PullRequest: truffleruby/3367

Author: eregon

spec/ruby/optional/capi/class_spec.rb

@@ -196,16 +196,6 @@
     end
   end
 
-  describe "rb_define_method" do
-    it "defines a method taking variable arguments as a C array if the argument count is -1" do
-      @s.rb_method_varargs_1(1, 3, 7, 4).should == [1, 3, 7, 4]
-    end
-
-    it "defines a method taking variable arguments as a Ruby array if the argument count is -2" do
-      @s.rb_method_varargs_2(1, 3, 7, 4).should == [1, 3, 7, 4]
-    end
-  end
-
   describe "rb_class2name" do
     it "returns the class name" do
       @s.rb_class2name(CApiClassSpecs).should == "CApiClassSpecs"

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

@@ -142,19 +142,6 @@ static VALUE class_spec_include_module(VALUE self, VALUE klass, VALUE module) {
   return klass;
 }
 
-static VALUE class_spec_method_var_args_1(int argc, VALUE *argv, VALUE self) {
-  VALUE ary = rb_ary_new();
-  int i;
-  for (i = 0; i < argc; i++) {
-    rb_ary_push(ary, argv[i]);
-  }
-  return ary;
-}
-
-static VALUE class_spec_method_var_args_2(VALUE self, VALUE argv) {
-  return argv;
-}
-
 void Init_class_spec(void) {
   VALUE cls = rb_define_class("CApiClassSpecs", rb_cObject);
   rb_define_method(cls, "define_call_super_method", class_spec_define_call_super_method, 2);
@@ -185,8 +172,6 @@ void Init_class_spec(void) {
   rb_define_method(cls, "rb_define_class_id_under", class_spec_rb_define_class_id_under, 3);
   rb_define_method(cls, "rb_define_class_variable", class_spec_define_class_variable, 3);
   rb_define_method(cls, "rb_include_module", class_spec_include_module, 2);
-  rb_define_method(cls, "rb_method_varargs_1", class_spec_method_var_args_1, -1);
-  rb_define_method(cls, "rb_method_varargs_2", class_spec_method_var_args_2, -2);
 }
 
 #ifdef __cplusplus

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

@@ -9,18 +9,6 @@ static VALUE module_specs_test_method(VALUE self) {
   return ID2SYM(rb_intern("test_method"));
 }
 
-static VALUE module_specs_test_method_2required(VALUE self, VALUE arg1, VALUE arg2) {
-    return ID2SYM(rb_intern("test_method_2required"));
-}
-
-static VALUE module_specs_test_method_c_array(int argc, VALUE *argv, VALUE self) {
-    return ID2SYM(rb_intern("test_method_c_array"));
-}
-
-static VALUE module_specs_test_method_ruby_array(VALUE self, VALUE args) {
-    return ID2SYM(rb_intern("test_method_ruby_array"));
-}
-
 static VALUE module_specs_const_defined(VALUE self, VALUE klass, VALUE id) {
   return rb_const_defined(klass, SYM2ID(id)) ? Qtrue : Qfalse;
 }
@@ -88,19 +76,25 @@ static VALUE module_specs_rb_define_method(VALUE self, VALUE cls, VALUE str_name
   return Qnil;
 }
 
-static VALUE module_specs_rb_define_method_2required(VALUE self, VALUE cls, VALUE str_name) {
-  rb_define_method(cls, RSTRING_PTR(str_name), module_specs_test_method_2required, 2);
-  return Qnil;
+static VALUE module_specs_method_var_args_1(int argc, VALUE *argv, VALUE self) {
+  VALUE ary = rb_ary_new();
+  int i;
+  for (i = 0; i < argc; i++) {
+    rb_ary_push(ary, argv[i]);
+  }
+  return ary;
 }
 
-static VALUE module_specs_rb_define_method_c_array(VALUE self, VALUE cls, VALUE str_name) {
-  rb_define_method(cls, RSTRING_PTR(str_name), module_specs_test_method_c_array, -1);
-  return Qnil;
+static VALUE module_specs_method_var_args_2(VALUE self, VALUE argv) {
+  return argv;
 }
 
-static VALUE module_specs_rb_define_method_ruby_array(VALUE self, VALUE cls, VALUE str_name) {
-  rb_define_method(cls, RSTRING_PTR(str_name), module_specs_test_method_ruby_array, -2);
-  return Qnil;
+static VALUE module_specs_rb_define_method_1required(VALUE self, VALUE arg1) {
+    return arg1;
+}
+
+static VALUE module_specs_rb_define_method_2required(VALUE self, VALUE arg1, VALUE arg2) {
+    return arg2;
 }
 
 static VALUE module_specs_rb_define_module_function(VALUE self, VALUE cls, VALUE str_name) {
@@ -155,25 +149,21 @@ void Init_module_spec(void) {
   rb_define_method(cls, "rb_define_module_under", module_specs_rb_define_module_under, 2);
   rb_define_method(cls, "rb_define_const", module_specs_define_const, 3);
   rb_define_method(cls, "rb_define_global_const", module_specs_define_global_const, 2);
-  rb_define_method(cls, "rb_define_global_function",
-      module_specs_rb_define_global_function, 1);
+  rb_define_method(cls, "rb_define_global_function", module_specs_rb_define_global_function, 1);
 
   rb_define_method(cls, "rb_define_method", module_specs_rb_define_method, 2);
+  rb_define_method(cls, "rb_define_method_varargs_1", module_specs_method_var_args_1, -1);
+  rb_define_method(cls, "rb_define_method_varargs_2", module_specs_method_var_args_2, -2);
+  rb_define_method(cls, "rb_define_method_1required", module_specs_rb_define_method_1required, 1);
   rb_define_method(cls, "rb_define_method_2required", module_specs_rb_define_method_2required, 2);
-  rb_define_method(cls, "rb_define_method_c_array", module_specs_rb_define_method_c_array, 2);
-  rb_define_method(cls, "rb_define_method_ruby_array", module_specs_rb_define_method_ruby_array, 2);
 
-  rb_define_method(cls, "rb_define_module_function",
-      module_specs_rb_define_module_function, 2);
+  rb_define_method(cls, "rb_define_module_function", module_specs_rb_define_module_function, 2);
 
-  rb_define_method(cls, "rb_define_private_method",
-      module_specs_rb_define_private_method, 2);
+  rb_define_method(cls, "rb_define_private_method", module_specs_rb_define_private_method, 2);
 
-  rb_define_method(cls, "rb_define_protected_method",
-      module_specs_rb_define_protected_method, 2);
+  rb_define_method(cls, "rb_define_protected_method", module_specs_rb_define_protected_method, 2);
 
-  rb_define_method(cls, "rb_define_singleton_method",
-      module_specs_rb_define_singleton_method, 2);
+  rb_define_method(cls, "rb_define_singleton_method", module_specs_rb_define_singleton_method, 2);
 
   rb_define_method(cls, "rb_undef_method", module_specs_rb_undef_method, 2);
   rb_define_method(cls, "rb_undef", module_specs_rb_undef, 2);

spec/ruby/optional/capi/module_spec.rb

@@ -252,22 +252,30 @@ def method_to_be_aliased
       cls.new.method(:test_method).arity.should == 0
     end
 
+    it "returns the correct arity when argc of the method in class is 1" do
+      @m.rb_define_method_1required(42).should == 42
+      @m.method(:rb_define_method_1required).arity.should == 1
+    end
+
+    it "returns the correct arity when argc of the method in class is 2" do
+      @m.rb_define_method_2required(1, 2).should == 2
+      @m.method(:rb_define_method_2required).arity.should == 2
+    end
+
+    it "defines a method taking variable arguments as a C array if the argument count is -1" do
+      @m.rb_define_method_varargs_1(1, 3, 7, 4).should == [1, 3, 7, 4]
+    end
+
     it "returns the correct arity when argc of the method in class is -1" do
-      cls = Class.new
-      @m.rb_define_method_c_array(cls, "test_method_c_array")
-      cls.new.method(:test_method_c_array).arity.should == -1
+      @m.method(:rb_define_method_varargs_1).arity.should == -1
     end
 
-    it "returns the correct arity when argc of the method in class is -2" do
-      cls = Class.new
-      @m.rb_define_method_ruby_array(cls, "test_method_ruby_array")
-      cls.new.method(:test_method_ruby_array).arity.should == -1
+    it "defines a method taking variable arguments as a Ruby array if the argument count is -2" do
+      @m.rb_define_method_varargs_2(1, 3, 7, 4).should == [1, 3, 7, 4]
     end
 
-    it "returns the correct arity when argc of the method in class is 2" do
-      cls = Class.new
-      @m.rb_define_method_2required(cls, "test_method_2required")
-      cls.new.method(:test_method_2required).arity.should == 2
+    it "returns the correct arity when argc of the method in class is -2" do
+      @m.method(:rb_define_method_varargs_2).arity.should == -1
     end
 
     it "defines a method on a module" do

spec/tags/optional/capi/module_tags.txt

@@ -1334,7 +1334,7 @@ private RubySymbol defineMethod(RubyModule module, String name, RubyProc proc,
                 MaterializedFrame callerFrame) {
             final RootCallTarget callTargetForLambda = proc.callTargets.getCallTargetForLambda();
             final RubyLambdaRootNode rootNode = RubyLambdaRootNode.of(callTargetForLambda);
-            final SharedMethodInfo info = proc.getSharedMethodInfo().forDefineMethod(module, name);
+            final SharedMethodInfo info = proc.getSharedMethodInfo().forDefineMethod(module, name, proc);
             final RubyNode body = rootNode.copyBody();
             final RubyNode newBody = new CallMethodWithLambdaBody(isSingleContext() ? proc : null,
                     callTargetForLambda, body);

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -12,6 +12,7 @@
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.klass.RubyClass;
 import org.truffleruby.core.module.RubyModule;
+import org.truffleruby.core.proc.RubyProc;
 import org.truffleruby.language.LexicalScope;
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.RubyGuards;
@@ -60,16 +61,16 @@ public SharedMethodInfo(
         this.argumentDescriptors = argumentDescriptors;
     }
 
-    public SharedMethodInfo forDefineMethod(RubyModule declaringModule, String methodName) {
+    public SharedMethodInfo forDefineMethod(RubyModule declaringModule, String methodName, RubyProc proc) {
         return new SharedMethodInfo(
                 sourceSection,
                 staticLexicalScope,
-                arity,
+                proc.arity,
                 methodName,
                 0, // no longer a block
                 moduleAndMethodName(declaringModule, methodName),
                 null,
-                argumentDescriptors);
+                proc.argumentDescriptors);
     }
 
     public SharedMethodInfo convertMethodMissingToMethod(RubyModule declaringModule, String methodName) {