зеркало из https://github.com/github/ruby.git
[Bug #20457] [Prism] Remove redundant return flag
This commit is contained in:
Родитель
7993b88eee
Коммит
644424941a
|
@ -735,11 +735,6 @@ flags:
|
|||
- name: FORCED_US_ASCII_ENCODING
|
||||
comment: "internal bytes forced the encoding to US-ASCII"
|
||||
comment: Flags for regular expression and match last line nodes.
|
||||
- name: ReturnNodeFlags
|
||||
values:
|
||||
- name: REDUNDANT
|
||||
comment: "a return statement that is redundant because it is the last statement in a method"
|
||||
comment: Flags for return nodes.
|
||||
- name: ShareableConstantNodeFlags
|
||||
values:
|
||||
- name: LITERAL
|
||||
|
@ -3455,7 +3450,6 @@ nodes:
|
|||
retry
|
||||
^^^^^
|
||||
- name: ReturnNode
|
||||
flags: ReturnNodeFlags
|
||||
fields:
|
||||
- name: keyword_loc
|
||||
type: location
|
||||
|
|
111
prism/prism.c
111
prism/prism.c
|
@ -3795,113 +3795,6 @@ pm_def_node_receiver_check(pm_parser_t *parser, const pm_node_t *node) {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* When a method body is created, we want to check if the last statement is a
|
||||
* return or a statement that houses a return. If it is, then we want to mark
|
||||
* that return as being redundant so that we can compile it differently but also
|
||||
* so that we can indicate that to the user.
|
||||
*/
|
||||
static void
|
||||
pm_def_node_body_redundant_return(pm_node_t *node) {
|
||||
switch (PM_NODE_TYPE(node)) {
|
||||
case PM_RETURN_NODE:
|
||||
node->flags |= PM_RETURN_NODE_FLAGS_REDUNDANT;
|
||||
break;
|
||||
case PM_BEGIN_NODE: {
|
||||
pm_begin_node_t *cast = (pm_begin_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL && cast->else_clause == NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_STATEMENTS_NODE: {
|
||||
pm_statements_node_t *cast = (pm_statements_node_t *) node;
|
||||
|
||||
if (cast->body.size > 0) {
|
||||
pm_def_node_body_redundant_return(cast->body.nodes[cast->body.size - 1]);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_IF_NODE: {
|
||||
pm_if_node_t *cast = (pm_if_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
|
||||
if (cast->consequent != NULL) {
|
||||
pm_def_node_body_redundant_return(cast->consequent);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_UNLESS_NODE: {
|
||||
pm_unless_node_t *cast = (pm_unless_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
|
||||
if (cast->consequent != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->consequent);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_ELSE_NODE: {
|
||||
pm_else_node_t *cast = (pm_else_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_CASE_NODE: {
|
||||
pm_case_node_t *cast = (pm_case_node_t *) node;
|
||||
pm_node_t *condition;
|
||||
|
||||
PM_NODE_LIST_FOREACH(&cast->conditions, index, condition) {
|
||||
pm_def_node_body_redundant_return(condition);
|
||||
}
|
||||
|
||||
if (cast->consequent != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->consequent);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_WHEN_NODE: {
|
||||
pm_when_node_t *cast = (pm_when_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_CASE_MATCH_NODE: {
|
||||
pm_case_match_node_t *cast = (pm_case_match_node_t *) node;
|
||||
pm_node_t *condition;
|
||||
|
||||
PM_NODE_LIST_FOREACH(&cast->conditions, index, condition) {
|
||||
pm_def_node_body_redundant_return(condition);
|
||||
}
|
||||
|
||||
if (cast->consequent != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->consequent);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case PM_IN_NODE: {
|
||||
pm_in_node_t *cast = (pm_in_node_t *) node;
|
||||
|
||||
if (cast->statements != NULL) {
|
||||
pm_def_node_body_redundant_return((pm_node_t *) cast->statements);
|
||||
}
|
||||
break;
|
||||
}
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Allocate and initialize a new DefNode node.
|
||||
*/
|
||||
|
@ -3934,10 +3827,6 @@ pm_def_node_create(
|
|||
pm_def_node_receiver_check(parser, receiver);
|
||||
}
|
||||
|
||||
if (body != NULL) {
|
||||
pm_def_node_body_redundant_return(body);
|
||||
}
|
||||
|
||||
*node = (pm_def_node_t) {
|
||||
{
|
||||
.type = PM_DEF_NODE,
|
||||
|
|
131
prism_compile.c
131
prism_compile.c
|
@ -5542,32 +5542,25 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
|
|||
const pm_node_location_t location = PM_NODE_START_LOCATION(parser, node);
|
||||
int lineno = (int) location.line;
|
||||
|
||||
if (PM_NODE_TYPE_P(node, PM_RETURN_NODE) && PM_NODE_FLAG_P(node, PM_RETURN_NODE_FLAGS_REDUNDANT) && ((const pm_return_node_t *) node)->arguments == NULL) {
|
||||
// If the node that we're compiling is a return node that is redundant,
|
||||
// then it cannot be considered a line node because the other parser
|
||||
// eliminates it from the parse tree. In this case we must replicate
|
||||
// this behavior.
|
||||
} else {
|
||||
if (PM_NODE_TYPE_P(node, PM_BEGIN_NODE) && (((const pm_begin_node_t *) node)->statements == NULL) && (((const pm_begin_node_t *) node)->rescue_clause != NULL)) {
|
||||
// If this node is a begin node and it has empty statements and also
|
||||
// has a rescue clause, then the other parser considers it as
|
||||
// starting on the same line as the rescue, as opposed to the
|
||||
// location of the begin keyword. We replicate that behavior here.
|
||||
lineno = (int) PM_NODE_START_LINE_COLUMN(parser, ((const pm_begin_node_t *) node)->rescue_clause).line;
|
||||
}
|
||||
if (PM_NODE_TYPE_P(node, PM_BEGIN_NODE) && (((const pm_begin_node_t *) node)->statements == NULL) && (((const pm_begin_node_t *) node)->rescue_clause != NULL)) {
|
||||
// If this node is a begin node and it has empty statements and also
|
||||
// has a rescue clause, then the other parser considers it as
|
||||
// starting on the same line as the rescue, as opposed to the
|
||||
// location of the begin keyword. We replicate that behavior here.
|
||||
lineno = (int) PM_NODE_START_LINE_COLUMN(parser, ((const pm_begin_node_t *) node)->rescue_clause).line;
|
||||
}
|
||||
|
||||
if (PM_NODE_FLAG_P(node, PM_NODE_FLAG_NEWLINE) && ISEQ_COMPILE_DATA(iseq)->last_line != lineno) {
|
||||
// If this node has the newline flag set and it is on a new line
|
||||
// from the previous nodes that have been compiled for this ISEQ,
|
||||
// then we need to emit a newline event.
|
||||
int event = RUBY_EVENT_LINE;
|
||||
if (PM_NODE_FLAG_P(node, PM_NODE_FLAG_NEWLINE) && ISEQ_COMPILE_DATA(iseq)->last_line != lineno) {
|
||||
// If this node has the newline flag set and it is on a new line
|
||||
// from the previous nodes that have been compiled for this ISEQ,
|
||||
// then we need to emit a newline event.
|
||||
int event = RUBY_EVENT_LINE;
|
||||
|
||||
ISEQ_COMPILE_DATA(iseq)->last_line = lineno;
|
||||
if (ISEQ_COVERAGE(iseq) && ISEQ_LINE_COVERAGE(iseq)) {
|
||||
event |= RUBY_EVENT_COVERAGE_LINE;
|
||||
}
|
||||
PUSH_TRACE(ret, event);
|
||||
ISEQ_COMPILE_DATA(iseq)->last_line = lineno;
|
||||
if (ISEQ_COVERAGE(iseq) && ISEQ_LINE_COVERAGE(iseq)) {
|
||||
event |= RUBY_EVENT_COVERAGE_LINE;
|
||||
}
|
||||
PUSH_TRACE(ret, event);
|
||||
}
|
||||
|
||||
switch (PM_NODE_TYPE(node)) {
|
||||
|
@ -8367,63 +8360,53 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
|
|||
const pm_return_node_t *cast = (const pm_return_node_t *) node;
|
||||
const pm_arguments_node_t *arguments = cast->arguments;
|
||||
|
||||
if (PM_NODE_FLAG_P(cast, PM_RETURN_NODE_FLAGS_REDUNDANT)) {
|
||||
enum rb_iseq_type type = ISEQ_BODY(iseq)->type;
|
||||
LABEL *splabel = 0;
|
||||
|
||||
const rb_iseq_t *parent_iseq = iseq;
|
||||
enum rb_iseq_type parent_type = ISEQ_BODY(parent_iseq)->type;
|
||||
while (parent_type == ISEQ_TYPE_RESCUE || parent_type == ISEQ_TYPE_ENSURE) {
|
||||
if (!(parent_iseq = ISEQ_BODY(parent_iseq)->parent_iseq)) break;
|
||||
parent_type = ISEQ_BODY(parent_iseq)->type;
|
||||
}
|
||||
|
||||
switch (parent_type) {
|
||||
case ISEQ_TYPE_TOP:
|
||||
case ISEQ_TYPE_MAIN:
|
||||
if (arguments) {
|
||||
PM_COMPILE_NOT_POPPED((const pm_node_t *) arguments);
|
||||
rb_warn("argument of top-level return is ignored");
|
||||
}
|
||||
else {
|
||||
PUSH_INSN(ret, location, putnil);
|
||||
if (parent_iseq == iseq) {
|
||||
type = ISEQ_TYPE_METHOD;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (type == ISEQ_TYPE_METHOD) {
|
||||
splabel = NEW_LABEL(0);
|
||||
PUSH_LABEL(ret, splabel);
|
||||
PUSH_ADJUST(ret, location, 0);
|
||||
}
|
||||
|
||||
if (arguments) {
|
||||
PM_COMPILE_NOT_POPPED((const pm_node_t *) arguments);
|
||||
}
|
||||
else {
|
||||
enum rb_iseq_type type = ISEQ_BODY(iseq)->type;
|
||||
LABEL *splabel = 0;
|
||||
PUSH_INSN(ret, location, putnil);
|
||||
}
|
||||
|
||||
const rb_iseq_t *parent_iseq = iseq;
|
||||
enum rb_iseq_type parent_type = ISEQ_BODY(parent_iseq)->type;
|
||||
while (parent_type == ISEQ_TYPE_RESCUE || parent_type == ISEQ_TYPE_ENSURE) {
|
||||
if (!(parent_iseq = ISEQ_BODY(parent_iseq)->parent_iseq)) break;
|
||||
parent_type = ISEQ_BODY(parent_iseq)->type;
|
||||
}
|
||||
|
||||
switch (parent_type) {
|
||||
case ISEQ_TYPE_TOP:
|
||||
case ISEQ_TYPE_MAIN:
|
||||
if (arguments) {
|
||||
rb_warn("argument of top-level return is ignored");
|
||||
}
|
||||
if (parent_iseq == iseq) {
|
||||
type = ISEQ_TYPE_METHOD;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (type == ISEQ_TYPE_METHOD) {
|
||||
splabel = NEW_LABEL(0);
|
||||
PUSH_LABEL(ret, splabel);
|
||||
PUSH_ADJUST(ret, location, 0);
|
||||
}
|
||||
|
||||
if (arguments) {
|
||||
PM_COMPILE_NOT_POPPED((const pm_node_t *) arguments);
|
||||
}
|
||||
else {
|
||||
PUSH_INSN(ret, location, putnil);
|
||||
}
|
||||
|
||||
if (type == ISEQ_TYPE_METHOD && can_add_ensure_iseq(iseq)) {
|
||||
pm_add_ensure_iseq(ret, iseq, 1, scope_node);
|
||||
PUSH_TRACE(ret, RUBY_EVENT_RETURN);
|
||||
PUSH_INSN(ret, location, leave);
|
||||
PUSH_ADJUST_RESTORE(ret, splabel);
|
||||
if (!popped) PUSH_INSN(ret, location, putnil);
|
||||
}
|
||||
else {
|
||||
PUSH_INSN1(ret, location, throw, INT2FIX(TAG_RETURN));
|
||||
if (popped) PUSH_INSN(ret, location, pop);
|
||||
}
|
||||
if (type == ISEQ_TYPE_METHOD && can_add_ensure_iseq(iseq)) {
|
||||
pm_add_ensure_iseq(ret, iseq, 1, scope_node);
|
||||
PUSH_TRACE(ret, RUBY_EVENT_RETURN);
|
||||
PUSH_INSN(ret, location, leave);
|
||||
PUSH_ADJUST_RESTORE(ret, splabel);
|
||||
if (!popped) PUSH_INSN(ret, location, putnil);
|
||||
}
|
||||
else {
|
||||
PUSH_INSN1(ret, location, throw, INT2FIX(TAG_RETURN));
|
||||
if (popped) PUSH_INSN(ret, location, pop);
|
||||
}
|
||||
|
||||
return;
|
||||
|
|
|
@ -1,73 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require_relative "../test_helper"
|
||||
|
||||
module Prism
|
||||
class RedundantReturnTest < TestCase
|
||||
def test_statements
|
||||
assert_redundant_return("def foo; return; end")
|
||||
refute_redundant_return("def foo; return; 1; end")
|
||||
end
|
||||
|
||||
def test_begin_implicit
|
||||
assert_redundant_return("def foo; return; rescue; end")
|
||||
refute_redundant_return("def foo; return; 1; rescue; end")
|
||||
refute_redundant_return("def foo; return; rescue; else; end")
|
||||
end
|
||||
|
||||
def test_begin_explicit
|
||||
assert_redundant_return("def foo; begin; return; rescue; end; end")
|
||||
refute_redundant_return("def foo; begin; return; 1; rescue; end; end")
|
||||
refute_redundant_return("def foo; begin; return; rescue; else; end; end")
|
||||
end
|
||||
|
||||
def test_if
|
||||
assert_redundant_return("def foo; return if bar; end")
|
||||
end
|
||||
|
||||
def test_unless
|
||||
assert_redundant_return("def foo; return unless bar; end")
|
||||
end
|
||||
|
||||
def test_else
|
||||
assert_redundant_return("def foo; if bar; baz; else; return; end; end")
|
||||
end
|
||||
|
||||
def test_case_when
|
||||
assert_redundant_return("def foo; case bar; when baz; return; end; end")
|
||||
end
|
||||
|
||||
def test_case_else
|
||||
assert_redundant_return("def foo; case bar; when baz; else; return; end; end")
|
||||
end
|
||||
|
||||
def test_case_match_in
|
||||
assert_redundant_return("def foo; case bar; in baz; return; end; end")
|
||||
end
|
||||
|
||||
def test_case_match_else
|
||||
assert_redundant_return("def foo; case bar; in baz; else; return; end; end")
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def assert_redundant_return(source)
|
||||
assert find_return(source).redundant?
|
||||
end
|
||||
|
||||
def refute_redundant_return(source)
|
||||
refute find_return(source).redundant?
|
||||
end
|
||||
|
||||
def find_return(source)
|
||||
queue = [Prism.parse(source).value]
|
||||
|
||||
while (current = queue.shift)
|
||||
return current if current.is_a?(ReturnNode)
|
||||
queue.concat(current.compact_child_nodes)
|
||||
end
|
||||
|
||||
flunk "Could not find return node in #{node.inspect}"
|
||||
end
|
||||
end
|
||||
end
|
|
@ -56,7 +56,7 @@
|
|||
│ │ ├── binary_operator: :*
|
||||
│ │ └── depth: 0
|
||||
│ └── @ ReturnNode (location: (4,2)-(4,10))
|
||||
│ ├── flags: newline, redundant
|
||||
│ ├── flags: newline
|
||||
│ ├── keyword_loc: (4,2)-(4,8) = "return"
|
||||
│ └── arguments:
|
||||
│ @ ArgumentsNode (location: (4,9)-(4,10))
|
||||
|
|
|
@ -27,7 +27,7 @@
|
|||
│ │ ├── flags: ∅
|
||||
│ │ └── body: (length: 1)
|
||||
│ │ └── @ ReturnNode (location: (3,10)-(3,19))
|
||||
│ │ ├── flags: newline, redundant
|
||||
│ │ ├── flags: newline
|
||||
│ │ ├── keyword_loc: (3,10)-(3,16) = "return"
|
||||
│ │ └── arguments:
|
||||
│ │ @ ArgumentsNode (location: (3,17)-(3,19))
|
||||
|
|
|
@ -29,7 +29,7 @@
|
|||
│ │ ├── flags: ∅
|
||||
│ │ └── body: (length: 1)
|
||||
│ │ └── @ ReturnNode (location: (2,0)-(2,6))
|
||||
│ │ ├── flags: newline, redundant
|
||||
│ │ ├── flags: newline
|
||||
│ │ ├── keyword_loc: (2,0)-(2,6) = "return"
|
||||
│ │ └── arguments: ∅
|
||||
│ ├── locals: [:b]
|
||||
|
|
Загрузка…
Ссылка в новой задаче