From af9b0da125871c6bfbc3b7893d199a79420e5833 Mon Sep 17 00:00:00 2001 From: mame Date: Sat, 4 Nov 2017 14:24:16 +0000 Subject: [PATCH] Avoid usage of the magic number `(NODE*)-1` This magic number has two meanings depending upon the context: * "required keyword argument (no name)" on NODE_LASGN (`def foo(x:)`) * "rest argument (no name)" on NODE_MASGN and NODE_POSTARG ('a, b, * = ary` or `a, b, *, z = ary`) To show this intention explicitly, two macros are introduced: NODE_SPECIAL_REQUIRED_KEYWORD and NODE_SPECIAL_NO_NAME_REST. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60650 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- compile.c | 6 +++--- node.c | 12 ++++++------ node.h | 3 +++ parse.y | 22 +++++++++++----------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/compile.c b/compile.c index 4bcbd0cec8..76bc107abe 100644 --- a/compile.c +++ b/compile.c @@ -3708,7 +3708,7 @@ compile_massign(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, const NODE *rhsn = node->nd_value; const NODE *splatn = node->nd_args; const NODE *lhsn = node->nd_head; - int lhs_splat = (splatn && (VALUE)splatn != (VALUE)-1) ? 1 : 0; + int lhs_splat = (splatn && splatn != NODE_SPECIAL_NO_NAME_REST) ? 1 : 0; if (!popped || splatn || !compile_massign_opt(iseq, ret, rhsn, lhsn)) { int llen = 0; @@ -3765,12 +3765,12 @@ compile_massign(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, const NODE *postn = splatn->nd_2nd; const NODE *restn = splatn->nd_1st; int num = (int)postn->nd_alen; - int flag = 0x02 | (((VALUE)restn == (VALUE)-1) ? 0x00 : 0x01); + int flag = 0x02 | ((restn == NODE_SPECIAL_NO_NAME_REST) ? 0x00 : 0x01); ADD_INSN2(ret, nd_line(splatn), expandarray, INT2FIX(num), INT2FIX(flag)); - if ((VALUE)restn != (VALUE)-1) { + if (restn != NODE_SPECIAL_NO_NAME_REST) { CHECK(compile_massign_lhs(iseq, ret, restn)); } while (postn) { diff --git a/node.c b/node.c index 71a6d18023..48fdf03c0a 100644 --- a/node.c +++ b/node.c @@ -366,12 +366,12 @@ dump_node(VALUE buf, VALUE indent, int comment, NODE *node) ANN("example: a, b = foo"); F_NODE(nd_value, "rhsn"); F_NODE(nd_head, "lhsn"); - if ((VALUE)node->nd_args != (VALUE)-1) { + if (node->nd_args != NODE_SPECIAL_NO_NAME_REST) { LAST_NODE; F_NODE(nd_args, "splatn"); } else { - F_MSG(nd_args, "splatn", "-1 (rest argument without name)"); + F_MSG(nd_args, "splatn", "NODE_SPECIAL_NO_NAME_REST (rest argument without name)"); } return; @@ -402,8 +402,8 @@ dump_node(VALUE buf, VALUE indent, int comment, NODE *node) asgn: F_ID(nd_vid, "variable"); LAST_NODE; - if (node->nd_value == (NODE *)-1) { - F_MSG(nd_value, "rvalue", "(required keyword argument)"); + if (node->nd_value == NODE_SPECIAL_REQUIRED_KEYWORD) { + F_MSG(nd_value, "rvalue", "NODE_SPECIAL_REQUIRED_KEYWORD (required keyword argument)"); } else { F_NODE(nd_value, "rvalue"); @@ -983,11 +983,11 @@ dump_node(VALUE buf, VALUE indent, int comment, NODE *node) ANN("post arguments"); ANN("format: *[nd_1st], [nd_2nd..] = .."); ANN("example: a, *rest, z = foo"); - if ((VALUE)node->nd_1st != (VALUE)-1) { + if (node->nd_1st != NODE_SPECIAL_NO_NAME_REST) { F_NODE(nd_1st, "rest argument"); } else { - F_MSG(nd_1st, "rest argument", "-1 (rest argument without name)"); + F_MSG(nd_1st, "rest argument", "NODE_SPECIAL_NO_NAME_REST (rest argument without name)"); } LAST_NODE; F_NODE(nd_2nd, "post arguments"); diff --git a/node.h b/node.h index 848ef1b76e..d85f86746a 100644 --- a/node.h +++ b/node.h @@ -445,6 +445,9 @@ typedef struct RNode { #define NEW_ATTRASGN(r,m,a) NEW_NODE(NODE_ATTRASGN,r,m,a) #define NEW_PRELUDE(p,b,o) NEW_NODE(NODE_PRELUDE,p,b,o) +#define NODE_SPECIAL_REQUIRED_KEYWORD ((NODE *)-1) +#define NODE_SPECIAL_NO_NAME_REST ((NODE *)-1) + RUBY_SYMBOL_EXPORT_BEGIN typedef struct node_buffer_struct node_buffer_t; diff --git a/parse.y b/parse.y index a608cfa556..e87a0fb22b 100644 --- a/parse.y +++ b/parse.y @@ -1804,7 +1804,7 @@ mlhs_basic : mlhs_head | mlhs_head tSTAR { /*%%%*/ - $$ = new_masgn($1, (NODE *)-1, &@1); + $$ = new_masgn($1, NODE_SPECIAL_NO_NAME_REST, &@1); /*% $$ = mlhs_add_star($1, Qnil); %*/ @@ -1812,7 +1812,7 @@ mlhs_basic : mlhs_head | mlhs_head tSTAR ',' mlhs_post { /*%%%*/ - $$ = new_masgn($1, new_postarg((NODE *)-1, $4, &@1), &@1); + $$ = new_masgn($1, new_postarg(NODE_SPECIAL_NO_NAME_REST, $4, &@1), &@1); /*% $1 = mlhs_add_star($1, Qnil); $$ = mlhs_add_post($1, $4); @@ -1838,7 +1838,7 @@ mlhs_basic : mlhs_head | tSTAR { /*%%%*/ - $$ = new_masgn(0, (NODE *)-1, &@1); + $$ = new_masgn(0, NODE_SPECIAL_NO_NAME_REST, &@1); /*% $$ = mlhs_add_star(mlhs_new(), Qnil); %*/ @@ -1846,7 +1846,7 @@ mlhs_basic : mlhs_head | tSTAR ',' mlhs_post { /*%%%*/ - $$ = new_masgn(0, new_postarg((NODE *)-1, $3, &@1), &@1); + $$ = new_masgn(0, new_postarg(NODE_SPECIAL_NO_NAME_REST, $3, &@1), &@1); /*% $$ = mlhs_add_star(mlhs_new(), Qnil); $$ = mlhs_add_post($$, $3); @@ -3353,7 +3353,7 @@ f_margs : f_marg_list | f_marg_list ',' tSTAR { /*%%%*/ - $$ = new_masgn($1, (NODE *)-1, &@1); + $$ = new_masgn($1, NODE_SPECIAL_NO_NAME_REST, &@1); /*% $$ = mlhs_add_star($1, Qnil); %*/ @@ -3361,7 +3361,7 @@ f_margs : f_marg_list | f_marg_list ',' tSTAR ',' f_marg_list { /*%%%*/ - $$ = new_masgn($1, new_postarg((NODE *)-1, $5, &@1), &@1); + $$ = new_masgn($1, new_postarg(NODE_SPECIAL_NO_NAME_REST, $5, &@1), &@1); /*% $$ = mlhs_add_star($1, Qnil); $$ = mlhs_add_post($$, $5); @@ -3389,7 +3389,7 @@ f_margs : f_marg_list | tSTAR { /*%%%*/ - $$ = new_masgn(0, (NODE *)-1, &@1); + $$ = new_masgn(0, NODE_SPECIAL_NO_NAME_REST, &@1); /*% $$ = mlhs_add_star(mlhs_new(), Qnil); %*/ @@ -3397,7 +3397,7 @@ f_margs : f_marg_list | tSTAR ',' f_marg_list { /*%%%*/ - $$ = new_masgn(0, new_postarg((NODE *)-1, $3, &@1), &@1); + $$ = new_masgn(0, new_postarg(NODE_SPECIAL_NO_NAME_REST, $3, &@1), &@1); /*% $$ = mlhs_add_star(mlhs_new(), Qnil); $$ = mlhs_add_post($$, $3); @@ -4753,7 +4753,7 @@ f_kw : f_label arg_value | f_label { current_arg = 0; - $$ = assignable($1, (NODE *)-1, &@1); + $$ = assignable($1, NODE_SPECIAL_REQUIRED_KEYWORD, &@1); /*%%%*/ $$ = new_kw_arg($$, &@1); /*% @@ -4773,7 +4773,7 @@ f_block_kw : f_label primary_value } | f_label { - $$ = assignable($1, (NODE *)-1, &@1); + $$ = assignable($1, NODE_SPECIAL_REQUIRED_KEYWORD, &@1); /*%%%*/ $$ = new_kw_arg($$, &@1); /*% @@ -10836,7 +10836,7 @@ new_args_tail_gen(struct parser_params *parser, NODE *k, ID kr, ID b, YYLTYPE *l NODE *val_node = kwn->nd_body->nd_value; ID vid = kwn->nd_body->nd_vid; - if (val_node == (NODE *)-1) { + if (val_node == NODE_SPECIAL_REQUIRED_KEYWORD) { vtable_add(required_kw_vars, vid); } else {