diff options
author | ko1 <ko1@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2014-11-27 10:15:47 +0000 |
---|---|---|
committer | ko1 <ko1@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2014-11-27 10:15:47 +0000 |
commit | 6115f65d7dd29561710c3e84bb27180e5bab4380 (patch) | |
tree | 637d955e548699811e54d3044b9bdfa4113a3ba2 | |
parent | 955c7f0698d2229f72ae9b9d2a49a1cbecb606e9 (diff) |
* 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
-rw-r--r-- | ChangeLog | 24 | ||||
-rw-r--r-- | class.c | 19 | ||||
-rw-r--r-- | test/ruby/test_keyword.rb | 15 | ||||
-rw-r--r-- | vm_args.c | 70 | ||||
-rw-r--r-- | vm_insnhelper.c | 8 |
5 files changed, 97 insertions, 39 deletions
@@ -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 @@ -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)); diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index d71108fd6f..d24cd5b0fc 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -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) @@ -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) { diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 7bb0aadaa8..edb0e0622d 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -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; |