diff --git a/compile.c b/compile.c index 099341bcf60635..e03d7663cce695 100644 --- a/compile.c +++ b/compile.c @@ -1867,7 +1867,7 @@ iseq_calc_param_size(rb_iseq_t *iseq) body->param.flags.has_kw || body->param.flags.has_kwrest) { - if (body->param.flags.has_block) { + if (body->param.flags.has_block && body->param.block_start != -1) { body->param.size = body->param.block_start + 1; } else if (body->param.flags.has_kwrest) { @@ -2009,7 +2009,7 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons if (node_args) { struct rb_iseq_constant_body *const body = ISEQ_BODY(iseq); - struct rb_args_info *args = &RNODE_ARGS(node_args)->nd_ainfo; + const struct rb_args_info *const args = &RNODE_ARGS(node_args)->nd_ainfo; ID rest_id = 0; int last_comma = 0; ID block_id = 0; @@ -2100,7 +2100,11 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons body->param.flags.accepts_no_kwarg = TRUE; } - if (block_id) { + if (args->no_blockarg) { + body->param.block_start = -1; + body->param.flags.has_block = TRUE; + } + else if (block_id) { body->param.block_start = arg_size++; body->param.flags.has_block = TRUE; } diff --git a/parse.y b/parse.y index a9db3d4e6eb181..7ba5a530636aaf 100644 --- a/parse.y +++ b/parse.y @@ -6846,6 +6846,11 @@ f_block_arg : blkarg_mark tIDENTIFIER $$ = $2; /*% ripper: blockarg!($:2) %*/ } + | blkarg_mark keyword_nil + { + $$ = idNil; + /*% ripper: blockarg!(ID2VAL(idNil)) %*/ + } | blkarg_mark { arg_var(p, idFWD_BLOCK); @@ -15011,6 +15016,10 @@ new_args_tail(struct parser_params *p, rb_node_kw_arg_t *kw_args, ID kw_rest_arg struct rb_args_info *args = &node->nd_ainfo; if (p->error_p) return node; + if (block == idNil) { + block = 0; + args->no_blockarg = TRUE; + } args->block_arg = block; args->kw_args = kw_args; diff --git a/prism/config.yml b/prism/config.yml index 934574b3eac21d..292aeb206bfe82 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -2662,6 +2662,18 @@ nodes: nil ^^^ + - name: NoBlockParameterNode + fields: + - name: operator_loc + type: location + - name: keyword_loc + type: location + comment: | + Represents the use of `&nil` inside method arguments. + + def a(&nil) + ^^^^ + end - name: NoKeywordsParameterNode fields: - name: operator_loc @@ -2806,7 +2818,6 @@ nodes: - NoKeywordsParameterNode - name: block type: node? - kind: BlockParameterNode comment: | Represents the list of parameters on a method, block, or lambda definition. diff --git a/prism/prism.c b/prism/prism.c index 38390b5a119dd3..2fe3ee69758e41 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -5060,6 +5060,30 @@ pm_nil_node_create(pm_parser_t *parser, const pm_token_t *token) { return node; } +/** + * Allocate and initialize a new NoKeywordsParameterNode node. + */ +static pm_no_block_parameter_node_t * +pm_no_block_parameter_node_create(pm_parser_t *parser, const pm_token_t *operator, const pm_token_t *keyword) { + assert(operator->type == PM_TOKEN_UAMPERSAND || operator->type == PM_TOKEN_AMPERSAND); + assert(keyword->type == PM_TOKEN_KEYWORD_NIL); + pm_no_block_parameter_node_t *node = PM_ALLOC_NODE(parser, pm_no_block_parameter_node_t); + + *node = (pm_no_block_parameter_node_t) { + { + .type = PM_NO_BLOCK_PARAMETER_NODE, + .location = { + .start = operator->start, + .end = keyword->end + } + }, + .operator_loc = PM_LOCATION_TOKEN_VALUE(operator), + .keyword_loc = PM_LOCATION_TOKEN_VALUE(keyword) + }; + + return node; +} + /** * Allocate and initialize a new NoKeywordsParameterNode node. */ @@ -5317,9 +5341,9 @@ pm_parameters_node_keyword_rest_set(pm_parameters_node_t *params, pm_node_t *par * Set the block parameter on a ParametersNode node. */ static void -pm_parameters_node_block_set(pm_parameters_node_t *params, pm_block_parameter_node_t *param) { +pm_parameters_node_block_set(pm_parameters_node_t *params, pm_node_t *param) { assert(params->block == NULL); - pm_parameters_node_location_set(params, (pm_node_t *) param); + pm_parameters_node_location_set(params, param); params->block = param; } @@ -12925,27 +12949,34 @@ parse_parameters( parser_lex(parser); pm_token_t operator = parser->previous; - pm_token_t name; + pm_node_t *param; - bool repeated = false; - if (accept1(parser, PM_TOKEN_IDENTIFIER)) { - name = parser->previous; - repeated = pm_parser_parameter_name_check(parser, &name); - pm_parser_local_add_token(parser, &name); + if (accept1(parser, PM_TOKEN_KEYWORD_NIL)) { + param = (pm_node_t *) pm_no_block_parameter_node_create(parser, &operator, &parser->previous); } else { - name = not_provided(parser); - parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_FORWARDING_BLOCK; - } + pm_token_t name; - pm_block_parameter_node_t *param = pm_block_parameter_node_create(parser, &name, &operator); - if (repeated) { - pm_node_flag_set_repeated_parameter((pm_node_t *)param); + bool repeated = false; + if (accept1(parser, PM_TOKEN_IDENTIFIER)) { + name = parser->previous; + repeated = pm_parser_parameter_name_check(parser, &name); + pm_parser_local_add_token(parser, &name); + } else { + name = not_provided(parser); + parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_FORWARDING_BLOCK; + } + + param = (pm_node_t *) pm_block_parameter_node_create(parser, &name, &operator); + if (repeated) { + pm_node_flag_set_repeated_parameter(param); + } } + if (params->block == NULL) { pm_parameters_node_block_set(params, param); } else { - pm_parser_err_node(parser, (pm_node_t *) param, PM_ERR_PARAMETER_BLOCK_MULTI); - pm_parameters_node_posts_append(params, (pm_node_t *) param); + pm_parser_err_node(parser, param, PM_ERR_PARAMETER_BLOCK_MULTI); + pm_parameters_node_posts_append(params, param); } break; diff --git a/prism_compile.c b/prism_compile.c index 4e4ec40ec14145..78e709c7010983 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -6548,6 +6548,13 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, return; } + case PM_NO_BLOCK_PARAMETER_NODE: { + // def foo(&nil); end + // ^^^^ + ISEQ_BODY(iseq)->param.block_start = -1; + ISEQ_BODY(iseq)->param.flags.has_block = TRUE; + return; + } case PM_NO_KEYWORDS_PARAMETER_NODE: { // def foo(**nil); end // ^^^^^ @@ -7504,25 +7511,30 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, // def foo(a, (b, *c, d), e = 1, *f, g, (h, *i, j), k:, l: 1, **m, &n) // ^^ if (parameters_node->block) { - body->param.block_start = local_index; body->param.flags.has_block = true; + if (PM_NODE_TYPE(parameters_node->block) == PM_NO_BLOCK_PARAMETER_NODE) { + body->param.block_start = -1; + } + else { + body->param.block_start = local_index; - pm_constant_id_t name = ((pm_block_parameter_node_t *) parameters_node->block)->name; + pm_constant_id_t name = ((pm_block_parameter_node_t *) parameters_node->block)->name; - if (name) { - if (PM_NODE_FLAG_P(parameters_node->block, PM_PARAMETER_FLAGS_REPEATED_PARAMETER)) { - ID local = pm_constant_id_lookup(scope_node, name); - local_table_for_iseq->ids[local_index] = local; + if (name) { + if (PM_NODE_FLAG_P(parameters_node->block, PM_PARAMETER_FLAGS_REPEATED_PARAMETER)) { + ID local = pm_constant_id_lookup(scope_node, name); + local_table_for_iseq->ids[local_index] = local; + } + else { + pm_insert_local_index(name, local_index, index_lookup_table, local_table_for_iseq, scope_node); + } } else { - pm_insert_local_index(name, local_index, index_lookup_table, local_table_for_iseq, scope_node); + pm_insert_local_special(idAnd, local_index, index_lookup_table, local_table_for_iseq); } - } - else { - pm_insert_local_special(idAnd, local_index, index_lookup_table, local_table_for_iseq); - } - local_index++; + local_index++; + } } } diff --git a/rubyparser.h b/rubyparser.h index 9a809aa059f220..a3d1df971bb46a 100644 --- a/rubyparser.h +++ b/rubyparser.h @@ -787,6 +787,7 @@ struct rb_args_info { struct RNode_OPT_ARG *opt_args; unsigned int no_kwarg: 1; + unsigned int no_blockarg: 1; unsigned int ruby2_keywords: 1; unsigned int forwarding: 1; }; diff --git a/spec/ruby/language/block_spec.rb b/spec/ruby/language/block_spec.rb index 578d9cb3b02847..a34350d6d062a6 100644 --- a/spec/ruby/language/block_spec.rb +++ b/spec/ruby/language/block_spec.rb @@ -1068,4 +1068,15 @@ def all_kwrest(arg1, arg2, *rest, post1, post2, kw1: 1, kw2: 2, okw1:, okw2:, ** all_kwrest(:a, :b, :c, :d, :e, okw1: 'x', okw2: 'y') { 1 }.should == 1 end end + + ruby_version_is "3.4" do + it "works alongside disallowed block argument" do + no_block = eval <<-EOF + proc {|arg1, &nil| arg1} + EOF + + no_block.call(:a).should == :a + -> { no_block.call(:a) {} }.should raise_error(ArgumentError, 'no block accepted') + end + end end diff --git a/spec/ruby/language/method_spec.rb b/spec/ruby/language/method_spec.rb index 9abe4cde204ccd..3fbec4b0e2bc17 100644 --- a/spec/ruby/language/method_spec.rb +++ b/spec/ruby/language/method_spec.rb @@ -1127,6 +1127,18 @@ def m(a, b = nil, c = nil, d, e: nil, **f) result = m(1, {foo: :bar}) result.should == [1, nil, nil, {foo: :bar}, nil, {}] end + + ruby_version_is "3.4" do + evaluate <<-ruby do + def m(a, &nil); a end; + ruby + + m(1).should == 1 + + -> { m(1) {} }.should raise_error(ArgumentError, 'no block accepted') + -> { m(1, &proc {}) }.should raise_error(ArgumentError, 'no block accepted') + end + end end context 'when passing an empty keyword splat to a method that does not accept keywords' do diff --git a/test/prism/location_test.rb b/test/prism/location_test.rb index c7ce248b569f00..ac74a34cea26c0 100644 --- a/test/prism/location_test.rb +++ b/test/prism/location_test.rb @@ -646,6 +646,10 @@ def test_NilNode assert_location(NilNode, "nil") end + def test_NoBlockParameterNode + assert_location(NoBlockParameterNode, "def foo(&nil); end", 8...12) { |node| node.parameters.block } + end + def test_NoKeywordsParameterNode assert_location(NoKeywordsParameterNode, "def foo(**nil); end", 8...13) { |node| node.parameters.keyword_rest } end diff --git a/test/ruby/test_syntax.rb b/test/ruby/test_syntax.rb index 42108f955f0241..90025ea1923331 100644 --- a/test/ruby/test_syntax.rb +++ b/test/ruby/test_syntax.rb @@ -192,6 +192,49 @@ def test_argument_forwarding_with_anon_rest_kwrest_and_block assert_syntax_error("def f(...); g(x: 1, **); end", /no anonymous keyword rest parameter/) end + def test_no_block_argument_in_method + assert_valid_syntax("def f(&nil) end") + assert_valid_syntax("def f(a, &nil) end") + assert_valid_syntax("def f(*rest, &nil) end") + assert_valid_syntax("def f(*rest, p, &nil) end") + assert_valid_syntax("def f(a, *rest, &nil) end") + assert_valid_syntax("def f(a, *rest, p, &nil) end") + assert_valid_syntax("def f(a, k: nil, &nil) end") + assert_valid_syntax("def f(a, k: nil, **kw, &nil) end") + assert_valid_syntax("def f(a, *rest, k: nil, &nil) end") + assert_valid_syntax("def f(a, *rest, k: nil, **kw, &nil) end") + assert_valid_syntax("def f(a, *rest, p, k: nil, &nil) end") + assert_valid_syntax("def f(a, *rest, p, k: nil, **kw, &nil) end") + + obj = Object.new + obj.instance_eval "def f(&nil) end" + assert_raise_with_message(ArgumentError, /block accepted/) {obj.f {}} + assert_raise_with_message(ArgumentError, /block accepted/) {obj.f(&proc {})} + end + + def test_no_block_argument_in_block + assert_valid_syntax("proc do |&nil| end") + assert_valid_syntax("proc do |a, &nil| end") + assert_valid_syntax("proc do |*rest, &nil| end") + assert_valid_syntax("proc do |*rest, p, &nil| end") + assert_valid_syntax("proc do |a, *rest, &nil| end") + assert_valid_syntax("proc do |a, *rest, p, &nil| end") + assert_valid_syntax("proc do |a, k: nil, &nil| end") + assert_valid_syntax("proc do |a, k: nil, **kw, &nil| end") + assert_valid_syntax("proc do |a, *rest, k: nil, &nil| end") + assert_valid_syntax("proc do |a, *rest, k: nil, **kw, &nil| end") + assert_valid_syntax("proc do |a, *rest, p, k: nil, &nil| end") + assert_valid_syntax("proc do |a, *rest, p, k: nil, **kw, &nil| end") + + pr = eval "proc {|&nil|}" + assert_nil(pr.call) + assert_raise_with_message(ArgumentError, /block accepted/) {pr.call {}} + pr = eval "proc {|a, &nil| a}" + assert_nil(pr.call) + assert_equal(1, pr.call(1)) + assert_raise_with_message(ArgumentError, /block accepted/) {pr.call {}} + end + def test_newline_in_block_parameters bug = '[ruby-dev:45292]' ["", "a", "a, b"].product(["", ";x", [";", "x"]]) do |params| diff --git a/vm_args.c b/vm_args.c index d1a7695c6e39cc..92641e71fc1703 100644 --- a/vm_args.c +++ b/vm_args.c @@ -12,6 +12,8 @@ NORETURN(static void raise_argument_error(rb_execution_context_t *ec, const rb_i NORETURN(static void argument_arity_error(rb_execution_context_t *ec, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc)); NORETURN(static void argument_kw_error(rb_execution_context_t *ec, const rb_iseq_t *iseq, const char *error, const VALUE keys)); VALUE rb_keyword_error_new(const char *error, VALUE keys); /* class.c */ +static VALUE set_error_backtrace(rb_execution_context_t *ec, const rb_iseq_t *iseq, const VALUE exc); + static VALUE method_missing(rb_execution_context_t *ec, VALUE obj, ID id, int argc, const VALUE *argv, enum method_missing_reason call_status, int kw_splat); const rb_callable_method_entry_t *rb_resolve_refined_method_callable(VALUE refinements, const rb_callable_method_entry_t *me); @@ -855,11 +857,20 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co } if (ISEQ_BODY(iseq)->param.flags.has_block) { - if (ISEQ_BODY(iseq)->local_iseq == iseq) { + int block_start = ISEQ_BODY(iseq)->param.block_start; + if (block_start == -1) { + VALUE given_block; + args_setup_block_parameter(ec, calling, &given_block); + if (!NIL_P(given_block)) { + VALUE exc = rb_exc_new_cstr(rb_eArgError, "no block accepted"); + rb_exc_raise(set_error_backtrace(ec, iseq, exc)); + } + } + else if (ISEQ_BODY(iseq)->local_iseq == iseq) { /* Do nothing */ } else { - args_setup_block_parameter(ec, calling, locals + ISEQ_BODY(iseq)->param.block_start); + args_setup_block_parameter(ec, calling, locals + block_start); } } @@ -876,8 +887,8 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co return opt_pc; } -static void -raise_argument_error(rb_execution_context_t *ec, const rb_iseq_t *iseq, const VALUE exc) +static VALUE +set_error_backtrace(rb_execution_context_t *ec, const rb_iseq_t *iseq, const VALUE exc) { VALUE at; @@ -896,6 +907,13 @@ raise_argument_error(rb_execution_context_t *ec, const rb_iseq_t *iseq, const VA rb_ivar_set(exc, idBt_locations, at); rb_exc_set_backtrace(exc, at); + return exc; +} + +static void +raise_argument_error(rb_execution_context_t *ec, const rb_iseq_t *iseq, const VALUE exc) +{ + set_error_backtrace(ec, iseq, exc); rb_exc_raise(exc); }