Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix arg_forward without parentheses [Bug #18267] #5269

Merged
merged 2 commits into from
Dec 14, 2021
Merged

Fix arg_forward without parentheses [Bug #18267] #5269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbcfe45
Revert "Rename `in_kwarg` as `in_argdef` as unrelated to keywords"
nobu Nov 29, 2021
120c052
Fix arg_forward without parentheses [Bug #18267]
nobu Nov 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 41 additions & 19 deletions parse.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ enum shareability {

struct lex_context {
unsigned int in_defined: 1;
unsigned int in_kwarg: 1;
unsigned int in_argdef: 1;
unsigned int in_def: 1;
unsigned int in_class: 1;
Expand Down Expand Up @@ -1758,13 +1759,13 @@ expr : command_call
SET_LEX_STATE(EXPR_BEG|EXPR_LABEL);
p->command_start = FALSE;
$<ctxt>2 = p->ctxt;
p->ctxt.in_argdef = 1;
p->ctxt.in_kwarg = 1;
$<tbl>$ = push_pvtbl(p);
}
p_top_expr_body
{
pop_pvtbl(p, $<tbl>3);
p->ctxt.in_argdef = $<ctxt>2.in_argdef;
p->ctxt.in_kwarg = $<ctxt>2.in_kwarg;
/*%%%*/
$$ = NEW_CASE3($1, NEW_IN($4, 0, 0, &@4), &@$);
/*% %*/
Expand All @@ -1776,13 +1777,13 @@ expr : command_call
SET_LEX_STATE(EXPR_BEG|EXPR_LABEL);
p->command_start = FALSE;
$<ctxt>2 = p->ctxt;
p->ctxt.in_argdef = 1;
p->ctxt.in_kwarg = 1;
$<tbl>$ = push_pvtbl(p);
}
p_top_expr_body
{
pop_pvtbl(p, $<tbl>3);
p->ctxt.in_argdef = $<ctxt>1.in_argdef;
p->ctxt.in_kwarg = $<ctxt>1.in_kwarg;
/*%%%*/
$$ = NEW_CASE3($1, NEW_IN($4, NEW_TRUE(&@4), NEW_FALSE(&@4), &@4), &@$);
/*% %*/
Expand Down Expand Up @@ -1817,7 +1818,12 @@ defn_head : k_def def_name
}
;

defs_head : k_def singleton dot_or_colon {SET_LEX_STATE(EXPR_FNAME);} def_name
defs_head : k_def singleton dot_or_colon
{
SET_LEX_STATE(EXPR_FNAME);
p->ctxt.in_argdef = 1;
}
def_name
{
SET_LEX_STATE(EXPR_ENDFN|EXPR_LABEL); /* force for args */
$$ = $5;
Expand Down Expand Up @@ -3396,6 +3402,7 @@ k_module : keyword_module
k_def : keyword_def
{
token_info_push(p, "def", &@$);
p->ctxt.in_argdef = 1;
}
;

Expand Down Expand Up @@ -3591,6 +3598,8 @@ f_any_kwrest : f_kwrest
| f_no_kwarg {$$ = ID2VAL(idNil);}
;

f_eq : {p->ctxt.in_argdef = 0;} '=';

block_args_tail : f_block_kwarg ',' f_kwrest opt_f_block_arg
{
$$ = new_args_tail(p, $1, $3, $4, &@3);
Expand Down Expand Up @@ -3703,6 +3712,7 @@ block_param_def : '|' opt_bv_decl '|'
{
p->cur_arg = 0;
p->max_numparam = ORDINAL_PARAM;
p->ctxt.in_argdef = 0;
/*%%%*/
$$ = 0;
/*% %*/
Expand All @@ -3712,6 +3722,7 @@ block_param_def : '|' opt_bv_decl '|'
{
p->cur_arg = 0;
p->max_numparam = ORDINAL_PARAM;
p->ctxt.in_argdef = 0;
/*%%%*/
$$ = $2;
/*% %*/
Expand Down Expand Up @@ -3792,6 +3803,7 @@ lambda : tLAMBDA

f_larglist : '(' f_args opt_bv_decl ')'
{
p->ctxt.in_argdef = 0;
/*%%%*/
$$ = $2;
p->max_numparam = ORDINAL_PARAM;
Expand All @@ -3800,6 +3812,7 @@ f_larglist : '(' f_args opt_bv_decl ')'
}
| f_args
{
p->ctxt.in_argdef = 0;
/*%%%*/
if (!args_info_empty_p($1->nd_ainfo))
p->max_numparam = ORDINAL_PARAM;
Expand Down Expand Up @@ -4058,7 +4071,7 @@ p_case_body : keyword_in
SET_LEX_STATE(EXPR_BEG|EXPR_LABEL);
p->command_start = FALSE;
$<ctxt>1 = p->ctxt;
p->ctxt.in_argdef = 1;
p->ctxt.in_kwarg = 1;
$<tbl>$ = push_pvtbl(p);
}
{
Expand All @@ -4068,7 +4081,7 @@ p_case_body : keyword_in
{
pop_pktbl(p, $<tbl>3);
pop_pvtbl(p, $<tbl>2);
p->ctxt.in_argdef = $<ctxt>1.in_argdef;
p->ctxt.in_kwarg = $<ctxt>1.in_kwarg;
}
compstmt
p_cases
Expand Down Expand Up @@ -4242,12 +4255,12 @@ p_expr_basic : p_value
{
$<tbl>$ = push_pktbl(p);
$<ctxt>1 = p->ctxt;
p->ctxt.in_argdef = 0;
p->ctxt.in_kwarg = 0;
}
p_kwargs rbrace
{
pop_pktbl(p, $<tbl>2);
p->ctxt.in_argdef = $<ctxt>1.in_argdef;
p->ctxt.in_kwarg = $<ctxt>1.in_kwarg;
$$ = new_hash_pattern(p, Qnone, $3, &@$);
}
| tLBRACE rbrace
Expand Down Expand Up @@ -5122,6 +5135,7 @@ superclass : '<'
f_opt_paren_args: f_paren_args
| none
{
p->ctxt.in_argdef = 0;
$$ = new_args_tail(p, Qnone, Qnone, Qnone, &@0);
$$ = new_args(p, Qnone, Qnone, Qnone, Qnone, $$, &@0);
}
Expand All @@ -5135,18 +5149,21 @@ f_paren_args : '(' f_args rparen
/*% ripper: paren!($2) %*/
SET_LEX_STATE(EXPR_BEG);
p->command_start = TRUE;
p->ctxt.in_argdef = 0;
}
;

f_arglist : f_paren_args
| {
$<ctxt>$ = p->ctxt;
p->ctxt.in_kwarg = 1;
p->ctxt.in_argdef = 1;
SET_LEX_STATE(p->lex.state|EXPR_LABEL); /* force for args */
}
f_args term
{
p->ctxt.in_argdef = $<ctxt>1.in_argdef;
p->ctxt.in_kwarg = $<ctxt>1.in_kwarg;
p->ctxt.in_argdef = 0;
$$ = $2;
SET_LEX_STATE(EXPR_BEG);
p->command_start = TRUE;
Expand Down Expand Up @@ -5363,13 +5380,15 @@ f_label : tLABEL
arg_var(p, formal_argument(p, $1));
p->cur_arg = get_id($1);
p->max_numparam = ORDINAL_PARAM;
p->ctxt.in_argdef = 0;
$$ = $1;
}
;

f_kw : f_label arg_value
{
p->cur_arg = 0;
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = new_kw_arg(p, assignable(p, $1, $2, &@$), &@$);
/*% %*/
Expand All @@ -5378,6 +5397,7 @@ f_kw : f_label arg_value
| f_label
{
p->cur_arg = 0;
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = new_kw_arg(p, assignable(p, $1, NODE_SPECIAL_REQUIRED_KEYWORD, &@$), &@$);
/*% %*/
Expand All @@ -5387,13 +5407,15 @@ f_kw : f_label arg_value

f_block_kw : f_label primary_value
{
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = new_kw_arg(p, assignable(p, $1, $2, &@$), &@$);
/*% %*/
/*% ripper: rb_assoc_new(get_value(assignable(p, $1)), get_value($2)) %*/
}
| f_label
{
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = new_kw_arg(p, assignable(p, $1, NODE_SPECIAL_REQUIRED_KEYWORD, &@$), &@$);
/*% %*/
Expand Down Expand Up @@ -5464,19 +5486,21 @@ f_kwrest : kwrest_mark tIDENTIFIER
}
;

f_opt : f_arg_asgn '=' arg_value
f_opt : f_arg_asgn f_eq arg_value
{
p->cur_arg = 0;
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = NEW_OPT_ARG(0, assignable(p, $1, $3, &@$), &@$);
/*% %*/
/*% ripper: rb_assoc_new(get_value(assignable(p, $1)), get_value($3)) %*/
}
;

f_block_opt : f_arg_asgn '=' primary_value
f_block_opt : f_arg_asgn f_eq primary_value
{
p->cur_arg = 0;
p->ctxt.in_argdef = 1;
/*%%%*/
$$ = NEW_OPT_ARG(0, assignable(p, $1, $3, &@$), &@$);
/*% %*/
Expand Down Expand Up @@ -9293,7 +9317,7 @@ parser_yylex(struct parser_params *p)
dispatch_scan_event(p, tIGNORED_NL);
}
fallthru = FALSE;
if (!c && p->ctxt.in_argdef) {
if (!c && p->ctxt.in_kwarg) {
goto normal_newline;
}
goto retry;
Expand Down Expand Up @@ -9659,13 +9683,11 @@ parser_yylex(struct parser_params *p)
SET_LEX_STATE(EXPR_BEG);
if ((c = nextc(p)) == '.') {
if ((c = nextc(p)) == '.') {
if (p->ctxt.in_argdef) {
SET_LEX_STATE(EXPR_ENDARG);
return tBDOT3;
}
if (p->lex.paren_nest == 0 && looking_at_eol_p(p)) {
if (p->ctxt.in_argdef || /* def foo a, ... */
IS_lex_state_for(last_state, EXPR_ENDFN) || /* def foo ... */
0) {
SET_LEX_STATE(EXPR_ENDARG);
return tBDOT3;
}
rb_warn0("... at EOL, should be parenthesized?");
}
else if (p->lex.lpar_beg >= 0 && p->lex.lpar_beg+1 == p->lex.paren_nest) {
Expand Down
25 changes: 24 additions & 1 deletion test/ruby/test_syntax.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1587,7 +1587,15 @@ def test_tautological_condition
def test_argument_forwarding
assert_valid_syntax('def foo(...) bar(...) end')
assert_valid_syntax('def foo(...) end')
assert_valid_syntax('def foo(a, ...) bar(...) end')
assert_valid_syntax("def foo ...\n bar(...)\nend")
assert_valid_syntax("def foo a, ...\n bar(...)\nend")
assert_valid_syntax("def foo b = 1, ...\n bar(...)\nend")
assert_valid_syntax("def foo ...; bar(...); end")
assert_valid_syntax("def foo a, ...; bar(...); end")
assert_valid_syntax("def foo b = 1, ...; bar(...); end")
assert_valid_syntax("(def foo ...\n bar(...)\nend)")
assert_valid_syntax("(def foo ...; bar(...); end)")
assert_valid_syntax('def ==(...) end')
assert_valid_syntax('def [](...) end')
assert_valid_syntax('def nil(...) end')
Expand Down Expand Up @@ -1618,8 +1626,10 @@ def obj1.bar(*args, **kws, &block)
end
end
obj4 = obj1.clone
obj5 = obj1.clone
obj1.instance_eval('def foo(...) bar(...) end', __FILE__, __LINE__)
obj4.instance_eval("def foo ...\n bar(...)\n""end", __FILE__, __LINE__)
obj5.instance_eval("def foo ...; bar(...); end", __FILE__, __LINE__)

klass = Class.new {
def foo(*args, **kws, &block)
Expand Down Expand Up @@ -1648,7 +1658,7 @@ def obj3.bar(*args, &block)
end
obj3.instance_eval('def foo(...) bar(...) end', __FILE__, __LINE__)

[obj1, obj2, obj3, obj4].each do |obj|
[obj1, obj2, obj3, obj4, obj5].each do |obj|
assert_warning('') {
assert_equal([[1, 2, 3], {k1: 4, k2: 5}], obj.foo(1, 2, 3, k1: 4, k2: 5) {|*x| x})
}
Expand Down Expand Up @@ -1791,6 +1801,19 @@ def obj.bar(*args, **kws, &block)
assert_equal [[4, 2], {a: 1}], obj.foo(4, 2, a: 1)
assert_equal [[4, 2, 3], {a: 1}], obj.foo(4, 2, 3, a: 1)
assert_equal [[4, 2, 3], {a: 1}], obj.foo(4, 2, 3, a: 1){|args, kws| [args, kws]}

obj.singleton_class.send(:remove_method, :foo)
obj.instance_eval("def foo a, ...; bar(a, ...); end", __FILE__, __LINE__)
assert_equal [[4], {}], obj.foo(4)
assert_equal [[4, 2], {}], obj.foo(4, 2)
assert_equal [[4, 2, 3], {}], obj.foo(4, 2, 3)
assert_equal [[4], {a: 1}], obj.foo(4, a: 1)
assert_equal [[4, 2], {a: 1}], obj.foo(4, 2, a: 1)
assert_equal [[4, 2, 3], {a: 1}], obj.foo(4, 2, 3, a: 1)
assert_equal [[4, 2, 3], {a: 1}], obj.foo(4, 2, 3, a: 1){|args, kws| [args, kws]}

exp = eval("-> (a: nil) {a...1}")
assert_equal 0...1, exp.call(a: 0)
end

def test_cdhash
Expand Down