ruby-changes:47771
From: mame <ko1@a...>
Date: Thu, 14 Sep 2017 14:27:09 +0900 (JST)
Subject: [ruby-changes:47771] mame:r59889 (trunk): Introduce NODE_UNLESS for branch coverage
mame 2017-09-14 14:27:02 +0900 (Thu, 14 Sep 2017) New Revision: 59889 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=59889 Log: Introduce NODE_UNLESS for branch coverage `unless` statement was a syntactic sugar for `if` statement, which made the result of branch coverage hard to understand. Modified files: trunk/compile.c trunk/node.c trunk/node.h trunk/parse.y trunk/test/coverage/test_coverage.rb Index: parse.y =================================================================== --- parse.y (revision 59888) +++ parse.y (revision 59889) @@ -352,7 +352,8 @@ static NODE *cond_gen(struct parser_para https://github.com/ruby/ruby/blob/trunk/parse.y#L352 #define new_nil() NEW_NIL() static NODE *new_if_gen(struct parser_params*,NODE*,NODE*,NODE*); #define new_if(cc,left,right) new_if_gen(parser, (cc), (left), (right)) -#define new_unless(cc,left,right) new_if_gen(parser, (cc), (right), (left)) +static NODE *new_unless_gen(struct parser_params*,NODE*,NODE*,NODE*); +#define new_unless(cc,left,right) new_unless_gen(parser, (cc), (left), (right)) static NODE *logop_gen(struct parser_params*,enum node_type,NODE*,NODE*); #define logop(id,node1,node2) \ logop_gen(parser, ((id)==idAND||(id)==idANDOP)?NODE_AND:NODE_OR, \ @@ -9533,6 +9534,7 @@ value_expr_gen(struct parser_params *par https://github.com/ruby/ruby/blob/trunk/parse.y#L9534 break; case NODE_IF: + case NODE_UNLESS: if (!node->nd_body) { node = node->nd_else; break; @@ -9711,6 +9713,7 @@ reduce_nodes_gen(struct parser_params *p https://github.com/ruby/ruby/blob/trunk/parse.y#L9713 body = &node->nd_end->nd_head; break; case NODE_IF: + case NODE_UNLESS: if (subnodes(nd_body, nd_else)) break; return; case NODE_CASE: @@ -9914,6 +9917,14 @@ new_if_gen(struct parser_params *parser, https://github.com/ruby/ruby/blob/trunk/parse.y#L9917 } static NODE* +new_unless_gen(struct parser_params *parser, NODE *cc, NODE *left, NODE *right) +{ + if (!cc) return right; + cc = cond0(parser, cc, FALSE); + return newline_node(NEW_UNLESS(cc, left, right)); +} + +static NODE* logop_gen(struct parser_params *parser, enum node_type type, NODE *left, NODE *right) { value_expr(left); Index: node.c =================================================================== --- node.c (revision 59888) +++ node.c (revision 59889) @@ -200,6 +200,16 @@ dump_node(VALUE buf, VALUE indent, int c https://github.com/ruby/ruby/blob/trunk/node.c#L200 F_NODE(nd_else, "else clause"); break; + case NODE_UNLESS: + ANN("unless statement"); + ANN("format: unless [nd_cond] then [nd_body] else [nd_else] end"); + ANN("example: unless x == 1 then foo else bar end"); + F_NODE(nd_cond, "condition expr"); + F_NODE(nd_body, "then clause"); + LAST_NODE; + F_NODE(nd_else, "else clause"); + break; + case NODE_CASE: ANN("case statement"); ANN("format: case [nd_head]; [nd_body]; end"); Index: node.h =================================================================== --- node.h (revision 59888) +++ node.h (revision 59889) @@ -26,6 +26,8 @@ enum node_type { https://github.com/ruby/ruby/blob/trunk/node.h#L26 #define NODE_BLOCK NODE_BLOCK NODE_IF, #define NODE_IF NODE_IF + NODE_UNLESS, +#define NODE_UNLESS NODE_UNLESS NODE_CASE, #define NODE_CASE NODE_CASE NODE_WHEN, @@ -362,7 +364,7 @@ typedef struct RNode { https://github.com/ruby/ruby/blob/trunk/node.h#L364 #define NEW_SCOPE(a,b) NEW_NODE(NODE_SCOPE,local_tbl(),b,a) #define NEW_BLOCK(a) NEW_NODE(NODE_BLOCK,a,0,0) #define NEW_IF(c,t,e) NEW_NODE(NODE_IF,c,t,e) -#define NEW_UNLESS(c,t,e) NEW_IF(c,e,t) +#define NEW_UNLESS(c,t,e) NEW_NODE(NODE_UNLESS,c,t,e) #define NEW_CASE(h,b) NEW_NODE(NODE_CASE,h,b,0) #define NEW_WHEN(c,t,e) NEW_NODE(NODE_WHEN,c,t,e) #define NEW_OPT_N(b) NEW_NODE(NODE_OPT_N,0,b,0) Index: compile.c =================================================================== --- compile.c (revision 59888) +++ compile.c (revision 59889) @@ -4178,8 +4178,11 @@ number_literal_p(NODE *n) https://github.com/ruby/ruby/blob/trunk/compile.c#L4178 } static int -compile_if(rb_iseq_t *iseq, LINK_ANCHOR *const ret, NODE *node, int popped) +compile_if(rb_iseq_t *iseq, LINK_ANCHOR *const ret, NODE *node, int popped, const enum node_type type) { + NODE *node_body = type == NODE_IF ? node->nd_body : node->nd_else; + NODE *node_else = type == NODE_IF ? node->nd_else : node->nd_body; + const int line = nd_line(node); DECL_ANCHOR(cond_seq); DECL_ANCHOR(then_seq); @@ -4196,19 +4199,19 @@ compile_if(rb_iseq_t *iseq, LINK_ANCHOR https://github.com/ruby/ruby/blob/trunk/compile.c#L4199 compile_branch_condition(iseq, cond_seq, node->nd_cond, then_label, else_label); - CHECK(COMPILE_(then_seq, "then", node->nd_body, popped)); - CHECK(COMPILE_(else_seq, "else", node->nd_else, popped)); + CHECK(COMPILE_(then_seq, "then", node_body, popped)); + CHECK(COMPILE_(else_seq, "else", node_else, popped)); ADD_SEQ(ret, cond_seq); if (then_label->refcnt && else_label->refcnt) { - DECL_BRANCH_BASE(branches, line, "if"); + DECL_BRANCH_BASE(branches, line, type == NODE_IF ? "if" : "unless"); } if (then_label->refcnt) { ADD_LABEL(ret, then_label); if (else_label->refcnt) { - ADD_TRACE_BRANCH_COVERAGE(ret, node->nd_body ? nd_line(node->nd_body) : line, "then", branches); + ADD_TRACE_BRANCH_COVERAGE(ret, node_body ? nd_line(node_body) : line, type == NODE_IF ? "then" : "else", branches); } ADD_SEQ(ret, then_seq); end_label = NEW_LABEL(line); @@ -4218,7 +4221,7 @@ compile_if(rb_iseq_t *iseq, LINK_ANCHOR https://github.com/ruby/ruby/blob/trunk/compile.c#L4221 if (else_label->refcnt) { ADD_LABEL(ret, else_label); if (then_label->refcnt) { - ADD_TRACE_BRANCH_COVERAGE(ret, node->nd_else ? nd_line(node->nd_else) : line, "else", branches); + ADD_TRACE_BRANCH_COVERAGE(ret, node_else ? nd_line(node_else) : line, type == NODE_IF ? "else" : "then", branches); } ADD_SEQ(ret, else_seq); } @@ -4936,7 +4939,8 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L4939 break; } case NODE_IF: - CHECK(compile_if(iseq, ret, node, popped)); + case NODE_UNLESS: + CHECK(compile_if(iseq, ret, node, popped, type)); break; case NODE_CASE: CHECK(compile_case(iseq, ret, node, popped)); Index: test/coverage/test_coverage.rb =================================================================== --- test/coverage/test_coverage.rb (revision 59888) +++ test/coverage/test_coverage.rb (revision 59889) @@ -186,13 +186,19 @@ class TestCoverage < Test::Unit::TestCas https://github.com/ruby/ruby/blob/trunk/test/coverage/test_coverage.rb#L186 f.puts ' else' f.puts ' 1' f.puts ' end' + f.puts '' + f.puts ' unless x == 0' + f.puts ' 0' + f.puts ' else' + f.puts ' 1' + f.puts ' end' f.puts 'end' f.puts 'foo(0)' f.puts 'foo(0)' f.puts 'foo(1)' end - assert_in_out_err(%w[-W0 -rcoverage], <<-"end;", ["{:branches=>{[:if, 0, 2]=>{[:then, 1, 3]=>2, [:else, 2, 5]=>1}}}"], []) + assert_in_out_err(%w[-W0 -rcoverage], <<-"end;", ["{:branches=>{[:if, 0, 2]=>{[:then, 1, 3]=>2, [:else, 2, 5]=>1}, [:unless, 3, 8]=>{[:else, 4, 11]=>2, [:then, 5, 9]=>1}}}"], []) ENV["COVERAGE_EXPERIMENTAL_MODE"] = "true" Coverage.start(branches: true) tmp = Dir.pwd -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/