* vm_args.c: fix backtrace location for keyword related exceptions.

For example, the following program
    def foo(k1: 1); end # line 1
            foo(k2: 2)          # line 2
  causes "unknown keyword: k2 (ArgumentError)".
  Before this patch, the backtrace location is only line 2.
  However, error should be located at line 1 (over line 2 in
  stack trace). This patch fix this problem.
* class.c (rb_keyword_error_new): separate exception creation logic
  from rb_keyword_error(), to use in vm_args.c.
* vm_insnhelper.c (rb_arg_error_new): rename to rb_arity_error_new().
* vm_args.c (argument_arity_error): rename to argument_arity_error().
* vm_args.c (arugment_kw_error): added to fix backtrace.
* test/ruby/test_keyword.rb: add tests.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@48608 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
ko1 2014-11-27 10:15:47 +00:00
parent 955c7f0698
commit 6115f65d7d
5 changed files with 97 additions and 39 deletions

View File

@ -1,3 +1,27 @@
Thu Nov 27 19:04:50 2014 Koichi Sasada <ko1@atdot.net>
* vm_args.c: fix backtrace location for keyword related exceptions.
For example, the following program
def foo(k1: 1); end # line 1
foo(k2: 2) # line 2
causes "unknown keyword: k2 (ArgumentError)".
Before this patch, the backtrace location is only line 2.
However, error should be located at line 1 (over line 2 in
stack trace). This patch fix this problem.
* class.c (rb_keyword_error_new): separate exception creation logic
from rb_keyword_error(), to use in vm_args.c.
* vm_insnhelper.c (rb_arg_error_new): rename to rb_arity_error_new().
* vm_args.c (argument_arity_error): rename to argument_arity_error().
* vm_args.c (arugment_kw_error): added to fix backtrace.
* test/ruby/test_keyword.rb: add tests.
Thu Nov 27 17:31:58 2014 Nobuyoshi Nakada <nobu@ruby-lang.org>
* common.mk (prelude.c): no longer depends on miniruby, since not

19
class.c
View File

@ -1857,11 +1857,12 @@ rb_scan_args(int argc, const VALUE *argv, const char *fmt, ...)
return argc;
}
NORETURN(void rb_keyword_error(const char *error, VALUE keys));
void
rb_keyword_error(const char *error, VALUE keys)
VALUE
rb_keyword_error_new(const char *error, VALUE keys)
{
const char *msg = "";
VALUE error_message;
if (RARRAY_LEN(keys) == 1) {
keys = RARRAY_AREF(keys, 0);
}
@ -1869,7 +1870,17 @@ rb_keyword_error(const char *error, VALUE keys)
keys = rb_ary_join(keys, rb_usascii_str_new2(", "));
msg = "s";
}
rb_raise(rb_eArgError, "%s keyword%s: %"PRIsVALUE, error, msg, keys);
error_message = rb_sprintf("%s keyword%s: %"PRIsVALUE, error, msg, keys);
return rb_exc_new_str(rb_eArgError, error_message);
}
NORETURN(static void rb_keyword_error(const char *error, VALUE keys));
static void
rb_keyword_error(const char *error, VALUE keys)
{
rb_exc_raise(rb_keyword_error_new(error, keys));
}
NORETURN(static void unknown_keyword_error(VALUE hash, const ID *table, int keywords));

View File

@ -310,11 +310,16 @@ class TestKeywordArguments < Test::Unit::TestCase
feature7701 = '[ruby-core:51454] [Feature #7701] required keyword argument'
o = Object.new
assert_nothing_raised(SyntaxError, feature7701) do
eval("def o.foo(a:) a; end")
eval("def o.foo(a:) a; end", nil, "xyzzy")
eval("def o.bar(a:,**b) [a, b]; end")
end
assert_raise_with_message(ArgumentError, /missing keyword/, feature7701) {o.foo}
assert_raise_with_message(ArgumentError, /unknown keyword/, feature7701) {o.foo(a:0, b:1)}
begin
o.foo(a: 0, b: 1)
rescue => e
assert_equal('xyzzy', e.backtrace_locations[0].path)
end
assert_equal(42, o.foo(a: 42), feature7701)
assert_equal([[:keyreq, :a]], o.method(:foo).parameters, feature7701)
@ -362,10 +367,16 @@ class TestKeywordArguments < Test::Unit::TestCase
def test_block_required_keyword
feature7701 = '[ruby-core:51454] [Feature #7701] required keyword argument'
b = assert_nothing_raised(SyntaxError, feature7701) do
break eval("proc {|a:| a}", nil, __FILE__, __LINE__)
break eval("proc {|a:| a}", nil, 'xyzzy', __LINE__)
end
assert_raise_with_message(ArgumentError, /missing keyword/, feature7701) {b.call}
assert_raise_with_message(ArgumentError, /unknown keyword/, feature7701) {b.call(a:0, b:1)}
begin
b.call(a: 0, b: 1)
rescue => e
assert_equal('xyzzy', e.backtrace_locations[0].path)
end
assert_equal(42, b.call(a: 42), feature7701)
assert_equal([[:keyreq, :a]], b.parameters, feature7701)

View File

@ -8,6 +8,11 @@
**********************************************************************/
NORETURN(static void raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc));
NORETURN(static void argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc));
NORETURN(static void arugment_kw_error(rb_thread_t *th, const rb_iseq_t *iseq, const char *error, const VALUE keys));
VALUE rb_keyword_error_new(const char *error, VALUE keys); /* class.c */
struct args_info {
/* basic args info */
rb_call_info_t *ci;
@ -359,8 +364,6 @@ make_unused_kw_hash(const ID *passed_keywords, int passed_keyword_len, const VAL
return obj;
}
void rb_keyword_error(const char *error, VALUE keys);
static inline int
args_setup_kw_parameters_lookup(const ID key, VALUE *ptr, const ID * const passed_keywords, VALUE *passed_values, const int passed_keyword_len)
{
@ -401,7 +404,7 @@ args_setup_kw_parameters(VALUE* const passed_values, const int passed_keyword_le
}
}
if (missing) rb_keyword_error("missing", missing);
if (missing) arugment_kw_error(GET_THREAD(), iseq, "missing", missing);
for (di=0; i<key_num; i++, di++) {
if (args_setup_kw_parameters_lookup(acceptable_keywords[i], &locals[i], passed_keywords, passed_values, passed_keyword_len)) {
@ -442,7 +445,7 @@ args_setup_kw_parameters(VALUE* const passed_values, const int passed_keyword_le
else {
if (found != passed_keyword_len) {
VALUE keys = make_unused_kw_hash(passed_keywords, passed_keyword_len, passed_values, TRUE);
rb_keyword_error("unknown", keys);
arugment_kw_error(GET_THREAD(), iseq, "unknown", keys);
}
}
@ -495,29 +498,6 @@ fill_keys_values(st_data_t key, st_data_t val, st_data_t ptr)
return ST_CONTINUE;
}
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 min_argc, int max_argc)
{
rb_thread_t *th = GET_THREAD();
VALUE exc = rb_arg_error_new(miss_argc, min_argc, max_argc);
VALUE at;
if (iseq) {
vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD, Qnil /* self */, Qnil /* klass */, Qnil /* specval*/,
iseq->iseq_encoded, th->cfp->sp, 0 /* local_size */, 0 /* me */, 0 /* stack_max */);
at = rb_vm_backtrace_object();
vm_pop_frame(th);
}
else {
at = rb_vm_backtrace_object();
}
rb_iv_set(exc, "bt_locations", at);
rb_funcall(exc, rb_intern("set_backtrace"), 1, at);
rb_exc_raise(exc);
}
static int
setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, rb_call_info_t * const ci,
VALUE * const locals, const enum arg_setup_type arg_setup_type)
@ -599,7 +579,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r
args_extend(args, min_argc);
}
else {
argument_error(iseq, given_argc, min_argc, max_argc);
argument_arity_error(th, iseq, given_argc, min_argc, max_argc);
}
}
}
@ -619,7 +599,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r
given_argc = max_argc;
}
else {
argument_error(iseq, given_argc, min_argc, max_argc);
argument_arity_error(th, iseq, given_argc, min_argc, max_argc);
}
}
@ -684,6 +664,38 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r
return opt_pc;
}
static void
raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc)
{
VALUE at;
if (iseq) {
vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD, Qnil /* self */, Qnil /* klass */, Qnil /* specval*/,
iseq->iseq_encoded, th->cfp->sp, 0 /* local_size */, 0 /* me */, 0 /* stack_max */);
at = rb_vm_backtrace_object();
vm_pop_frame(th);
}
else {
at = rb_vm_backtrace_object();
}
rb_iv_set(exc, "bt_locations", at);
rb_funcall(exc, rb_intern("set_backtrace"), 1, at);
rb_exc_raise(exc);
}
static void
argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc)
{
raise_argument_error(th, iseq, rb_arity_error_new(miss_argc, min_argc, max_argc));
}
static void
arugment_kw_error(rb_thread_t *th, const rb_iseq_t *iseq, const char *error, const VALUE keys)
{
raise_argument_error(th, iseq, rb_keyword_error_new(error, keys));
}
static inline void
vm_caller_setup_arg_splat(rb_control_frame_t *cfp, rb_call_info_t *ci)
{

View File

@ -115,7 +115,7 @@ vm_pop_frame(rb_thread_t *th)
/* method dispatch */
static inline VALUE
rb_arg_error_new(int argc, int min, int max)
rb_arity_error_new(int argc, int min, int max)
{
VALUE err_mess = 0;
if (min == max) {
@ -133,7 +133,7 @@ rb_arg_error_new(int argc, int min, int max)
void
rb_error_arity(int argc, int min, int max)
{
rb_exc_raise(rb_arg_error_new(argc, min, max));
rb_exc_raise(rb_arity_error_new(argc, min, max));
}
/* svar */
@ -1064,7 +1064,7 @@ vm_callee_setup_block_arg(rb_thread_t *th, rb_call_info_t *ci, const rb_iseq_t *
ci->argc = vm_callee_setup_block_arg_arg0_splat(cfp, iseq, argv, arg0);
}
else {
argument_error(iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num);
argument_arity_error(th, iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num);
}
}
@ -1084,7 +1084,7 @@ vm_callee_setup_arg(rb_thread_t *th, rb_call_info_t *ci, const rb_iseq_t *iseq,
CALLER_SETUP_ARG(cfp, ci); /* splat arg */
if (ci->argc != iseq->param.lead_num) {
argument_error(iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num);
argument_arity_error(th, iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num);
}
ci->aux.opt_pc = 0;