* vm_insnhelper.c: improve number of arguments error in case of

  optional parameters (issue #6085)

* include/ruby/intern.h: define UNLIMITED_ARGUMENTS

* test/ruby/test_arity.rb:  test for above

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@35023 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 85 insertions, 21 deletions

diff --git a/include/ruby/intern.h b/include/ruby/intern.h
index 7c5d79d7b3..763472589b 100644
--- a/include/ruby/intern.h
+++ b/include/ruby/intern.h
@@ -43,6 +43,7 @@ extern "C" {
  */
 
 #define ID_ALLOCATOR 1
+#define UNLIMITED_ARGUMENTS (-1)
 
 /* array.c */
 void rb_mem_clear(register VALUE*, register long);
diff --git a/test/ruby/test_arity.rb b/test/ruby/test_arity.rb
new file mode 100644
index 0000000000..23c2cd9e67
--- /dev/null
+++ b/test/ruby/test_arity.rb
@@ -0,0 +1,61 @@
+require 'test/unit'
+
+class TestArity < Test::Unit::TestCase
+  def err_mess(method_proc = nil, argc = 0)
+    args = (1..argc).to_a
+    case method_proc
+      when nil
+        yield
+      when Symbol
+        method(method_proc).call(*args)
+      else
+        method_proc.call(*args)
+    end
+    fail "Expected ArgumentError to be raised"
+  rescue ArgumentError => err
+    s = err.to_s
+    assert s =~ /wrong number of arguments \((.*)\)/, "Unexpected ArgumentError's message: #{s}"
+    $1
+  end
+
+  def a
+  end
+
+  def b(a, b, c, d=1, e=2, f, g, h, i, &block)
+  end
+
+  def c(a, b, c, d=1, e=2, *rest)
+  end
+
+  def d(a, b: 42)
+  end
+
+  def e(a, b:42, **c)
+  end
+
+  def f(a, b, c=1, *rest, d: 3)
+  end
+
+  def test_method_err_mess
+    assert_equal "1 for 0",     err_mess(:a, 1)
+    assert_equal "10 for 7..9", err_mess(:b, 10)
+    assert_equal "2 for 3+",    err_mess(:c, 2)
+    assert_equal "2 for 1",     err_mess(:d, 2)
+    assert_equal "0 for 1",     err_mess(:d, 0)
+    assert_equal "2 for 1",     err_mess(:e, 2)
+    assert_equal "0 for 1",     err_mess(:e, 0)
+    assert_equal "1 for 2+",    err_mess(:f, 1)
+  end
+
+  def test_proc_err_mess
+    assert_equal "0 for 1..2",  err_mess(->(b, c=42){}, 0)
+    assert_equal "1 for 2+",    err_mess(->(a, b, c=42, *d){}, 1)
+    assert_equal "3 for 4+",    err_mess(->(a, b, *c, d, e){}, 3)
+    assert_equal "3 for 1..2",  err_mess(->(b, c=42){}, 3)
+    assert_equal "1 for 0",     err_mess(->(&block){}, 1)
+    # Double checking:
+    p = Proc.new{|b, c=42| :ok}
+    assert_equal :ok,  p.call(1, 2, 3)
+    assert_equal :ok,  p.call
+  end
+end
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 5f7de4a199..f471ca5ccb 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -105,13 +105,26 @@ vm_pop_frame(rb_thread_t *th)
 }
 
 /* method dispatch */
+static inline VALUE
+rb_arg_error_new(int argc, int min, int max) {
+    VALUE err_mess = 0;
+    if (min == max) {
+	err_mess = rb_sprintf("wrong number of arguments (%d for %d)", argc, min);
+    }
+    else if (max == UNLIMITED_ARGUMENTS) {
+	err_mess = rb_sprintf("wrong number of arguments (%d for %d+)", argc, min);
+    }
+    else {
+	err_mess = rb_sprintf("wrong number of arguments (%d for %d..%d)", argc, min, max);
+    }
+    return rb_exc_new3(rb_eArgError, err_mess);
+}
 
-NORETURN(static void argument_error(const rb_iseq_t *iseq, int miss_argc, int correct_argc));
+NORETURN(static void argument_error(const rb_iseq_t *iseq, int miss_argc, int min_argc, int max_argc));
 static void
-argument_error(const rb_iseq_t *iseq, int miss_argc, int correct_argc)
+argument_error(const rb_iseq_t *iseq, int miss_argc, int min_argc, int max_argc)
 {
-    VALUE mesg = rb_sprintf("wrong number of arguments (%d for %d)", miss_argc, correct_argc);
-    VALUE exc = rb_exc_new3(rb_eArgError, mesg);
+    VALUE exc = rb_arg_error_new(miss_argc, min_argc, max_argc);
     VALUE bt = rb_make_backtrace();
     VALUE err_line = 0;
 
@@ -154,7 +167,7 @@ unknown_keyword_error(const rb_iseq_t *iseq, VALUE hash)
     if (LIKELY((iseq)->arg_simple & 0x01)) { \
 	/* simple check */ \
 	if ((orig_argc) != (iseq)->argc) { \
-	    argument_error((iseq), (orig_argc), (iseq)->argc); \
+	    argument_error((iseq), (orig_argc), (iseq)->argc, (iseq)->argc); \
 	} \
 	(ret) = 0; \
     } \
@@ -168,6 +181,9 @@ vm_callee_setup_arg_complex(rb_thread_t *th, const rb_iseq_t * iseq,
 			    const rb_block_t **block)
 {
     const int m = iseq->argc;
+    const int opts = iseq->arg_opts - (iseq->arg_opts > 0);
+    const int min = m + iseq->arg_post_len;
+    const int max = (iseq->arg_rest == -1) ? m + opts + iseq->arg_post_len : UNLIMITED_ARGUMENTS;
     int argc = orig_argc;
     VALUE *argv = orig_argv;
     rb_num_t opt_pc = 0;
@@ -196,8 +212,8 @@ vm_callee_setup_arg_complex(rb_thread_t *th, const rb_iseq_t * iseq,
     }
 
     /* mandatory */
-    if (argc < (m + iseq->arg_post_len)) { /* check with post arg */
-	argument_error(iseq, argc, m + iseq->arg_post_len);
+    if ((argc < min) || (argc > max && max != UNLIMITED_ARGUMENTS)) {
+	argument_error(iseq, argc, min, max);
     }
 
     argv += m;
@@ -217,12 +233,6 @@ vm_callee_setup_arg_complex(rb_thread_t *th, const rb_iseq_t * iseq,
 
     /* opt arguments */
     if (iseq->arg_opts) {
-	const int opts = iseq->arg_opts - 1 /* no opt */;
-
-	if (iseq->arg_rest == -1 && argc > opts) {
-	    argument_error(iseq, orig_argc, m + opts + iseq->arg_post_len);
-	}
-
 	if (argc > opts) {
 	    argc -= opts;
 	    argv += opts;
@@ -254,10 +264,6 @@ vm_callee_setup_arg_complex(rb_thread_t *th, const rb_iseq_t * iseq,
 	VALUE blockval = Qnil;
 	const rb_block_t *blockptr = *block;
 
-	if (argc != 0) {
-	    argument_error(iseq, orig_argc, m + iseq->arg_post_len);
-	}
-
 	if (blockptr) {
 	    /* make Proc object */
 	    if (blockptr->proc == 0) {
@@ -274,10 +280,6 @@ vm_callee_setup_arg_complex(rb_thread_t *th, const rb_iseq_t * iseq,
 	orig_argv[iseq->arg_block] = blockval; /* Proc or nil */
     }
 
-    if (iseq->arg_keyword && argc != 0) {
-	argument_error(iseq, orig_argc, m + iseq->arg_post_len);
-    }
-
     th->mark_stack_len = 0;
     return (int)opt_pc;
 }