Skip to content

Commit fe20830

Browse files
kddnewtonkddnewton
committed
Consistently place block arguments on block slot for call
1 parent b700c29 commit fe20830

17 files changed

+676
-672
lines changed

‎config.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,6 @@ nodes:
668668
type: location?
669669
- name: block
670670
type: node?
671-
kind: BlockNode
672671
- name: flags
673672
type: flags
674673
kind: CallNodeFlags
@@ -2315,7 +2314,6 @@ nodes:
23152314
type: location?
23162315
- name: block
23172316
type: node?
2318-
kind: BlockNode
23192317
comment: |
23202318
Represents the use of the `super` keyword with parentheses or arguments.
23212319

‎src/yarp.c

Lines changed: 91 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -549,48 +549,45 @@ typedef struct {
549549
yp_location_t opening_loc;
550550
yp_arguments_node_t *arguments;
551551
yp_location_t closing_loc;
552-
yp_block_node_t *block;
553-
554-
// This boolean is used to tell if there is an implicit block (i.e., an
555-
// argument passed with an & operator).
556-
bool implicit_block;
552+
yp_node_t *block;
557553
} yp_arguments_t;
558554

559555
#define YP_EMPTY_ARGUMENTS ((yp_arguments_t) { \
560556
.opening_loc = YP_OPTIONAL_LOCATION_NOT_PROVIDED_VALUE, \
561557
.arguments = NULL, \
562558
.closing_loc = YP_OPTIONAL_LOCATION_NOT_PROVIDED_VALUE, \
563559
.block = NULL, \
564-
.implicit_block = false \
565560
})
566561

567-
// Check that the set of arguments parsed for a given node is valid. This means
568-
// checking that we don't have both an implicit and explicit block.
562+
// Check that we're not about to attempt to attach a brace block to a call that
563+
// has arguments without parentheses.
569564
static void
570-
yp_arguments_validate(yp_parser_t *parser, yp_arguments_t *arguments) {
571-
if (arguments->block != NULL && arguments->implicit_block) {
572-
yp_diagnostic_list_append(
573-
&parser->error_list,
574-
arguments->block->base.location.start,
575-
arguments->block->base.location.end,
576-
YP_ERR_ARGUMENT_BLOCK_MULTI
577-
);
565+
yp_arguments_validate_block(yp_parser_t *parser, yp_arguments_t *arguments, yp_block_node_t *block) {
566+
// First, check that we have arguments and that we don't have a closing
567+
// location for them.
568+
if (arguments->arguments == NULL || arguments->closing_loc.start != NULL) {
569+
return;
578570
}
579571

580-
// Brace blocks can't be attached to arguments, only to the call
581-
if (arguments->block != NULL &&
582-
*arguments->block->opening_loc.start == '{' &&
583-
arguments->arguments != NULL &&
584-
!(arguments->arguments->arguments.size == 1 &&
585-
YP_NODE_TYPE_P(arguments->arguments->arguments.nodes[0], YP_PARENTHESES_NODE)) &&
586-
!arguments->closing_loc.end) {
587-
yp_diagnostic_list_append(
588-
&parser->error_list,
589-
arguments->block->base.location.start,
590-
arguments->block->base.location.end,
591-
YP_ERR_ARGUMENT_BLOCK_MULTI
592-
);
572+
// Next, check that we don't have a single parentheses argument. This would
573+
// look like:
574+
//
575+
// foo (1) {}
576+
//
577+
// In this case, it's actually okay for the block to be attached to the
578+
// call, even though it looks like it's attached to the argument.
579+
if (arguments->arguments->arguments.size == 1 && YP_NODE_TYPE_P(arguments->arguments->arguments.nodes[0], YP_PARENTHESES_NODE)) {
580+
return;
593581
}
582+
583+
// If we didn't hit a case before this check, then at this point we need to
584+
// add a syntax error.
585+
yp_diagnostic_list_append(
586+
&parser->error_list,
587+
block->base.location.start,
588+
block->base.location.end,
589+
YP_ERR_ARGUMENT_UNEXPECTED_BLOCK
590+
);
594591
}
595592

596593
/******************************************************************************/
@@ -1340,7 +1337,7 @@ yp_call_node_aref_create(yp_parser_t *parser, yp_node_t *receiver, yp_arguments_
13401337

13411338
node->base.location.start = receiver->location.start;
13421339
if (arguments->block != NULL) {
1343-
node->base.location.end = arguments->block->base.location.end;
1340+
node->base.location.end = arguments->block->location.end;
13441341
} else {
13451342
node->base.location.end = arguments->closing_loc.end;
13461343
}
@@ -1384,7 +1381,7 @@ yp_call_node_call_create(yp_parser_t *parser, yp_node_t *receiver, yp_token_t *o
13841381

13851382
node->base.location.start = receiver->location.start;
13861383
if (arguments->block != NULL) {
1387-
node->base.location.end = arguments->block->base.location.end;
1384+
node->base.location.end = arguments->block->location.end;
13881385
} else if (arguments->closing_loc.start != NULL) {
13891386
node->base.location.end = arguments->closing_loc.end;
13901387
} else if (arguments->arguments != NULL) {
@@ -1417,7 +1414,7 @@ yp_call_node_fcall_create(yp_parser_t *parser, yp_token_t *message, yp_arguments
14171414

14181415
node->base.location.start = message->start;
14191416
if (arguments->block != NULL) {
1420-
node->base.location.end = arguments->block->base.location.end;
1417+
node->base.location.end = arguments->block->location.end;
14211418
} else if (arguments->closing_loc.start != NULL) {
14221419
node->base.location.end = arguments->closing_loc.end;
14231420
} else if (arguments->arguments != NULL) {
@@ -1465,7 +1462,7 @@ yp_call_node_shorthand_create(yp_parser_t *parser, yp_node_t *receiver, yp_token
14651462

14661463
node->base.location.start = receiver->location.start;
14671464
if (arguments->block != NULL) {
1468-
node->base.location.end = arguments->block->base.location.end;
1465+
node->base.location.end = arguments->block->location.end;
14691466
} else {
14701467
node->base.location.end = arguments->closing_loc.end;
14711468
}
@@ -2414,18 +2411,24 @@ yp_forwarding_parameter_node_create(yp_parser_t *parser, const yp_token_t *token
24142411
// Allocate and initialize a new ForwardingSuper node.
24152412
static yp_forwarding_super_node_t *
24162413
yp_forwarding_super_node_create(yp_parser_t *parser, const yp_token_t *token, yp_arguments_t *arguments) {
2414+
assert(arguments->block == NULL || YP_NODE_TYPE_P(arguments->block, YP_BLOCK_NODE));
24172415
assert(token->type == YP_TOKEN_KEYWORD_SUPER);
24182416
yp_forwarding_super_node_t *node = YP_ALLOC_NODE(parser, yp_forwarding_super_node_t);
24192417

2418+
yp_block_node_t *block = NULL;
2419+
if (arguments->block != NULL) {
2420+
block = (yp_block_node_t *) arguments->block;
2421+
}
2422+
24202423
*node = (yp_forwarding_super_node_t) {
24212424
{
24222425
.type = YP_FORWARDING_SUPER_NODE,
24232426
.location = {
24242427
.start = token->start,
2425-
.end = arguments->block != NULL ? arguments->block->base.location.end : token->end
2428+
.end = block != NULL ? block->base.location.end : token->end
24262429
},
24272430
},
2428-
.block = arguments->block
2431+
.block = block
24292432
};
24302433

24312434
return node;
@@ -4282,7 +4285,7 @@ yp_super_node_create(yp_parser_t *parser, const yp_token_t *keyword, yp_argument
42824285

42834286
const uint8_t *end;
42844287
if (arguments->block != NULL) {
4285-
end = arguments->block->base.location.end;
4288+
end = arguments->block->location.end;
42864289
} else if (arguments->closing_loc.start != NULL) {
42874290
end = arguments->closing_loc.end;
42884291
} else if (arguments->arguments != NULL) {
@@ -8893,6 +8896,16 @@ parse_assocs(yp_parser_t *parser, yp_node_t *node) {
88938896
}
88948897
}
88958898

8899+
// Append an argument to a list of arguments.
8900+
static inline void
8901+
parse_arguments_append(yp_parser_t *parser, yp_arguments_t *arguments, yp_node_t *argument) {
8902+
if (arguments->arguments == NULL) {
8903+
arguments->arguments = yp_arguments_node_create(parser);
8904+
}
8905+
8906+
yp_arguments_node_arguments_append(arguments->arguments, argument);
8907+
}
8908+
88968909
// Parse a list of arguments.
88978910
static void
88988911
parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_forwarding, yp_token_type_t terminator) {
@@ -8926,13 +8939,14 @@ parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_for
89268939
}
89278940

89288941
yp_keyword_hash_node_t *hash = yp_keyword_hash_node_create(parser);
8929-
argument = (yp_node_t *)hash;
8942+
argument = (yp_node_t *) hash;
89308943

89318944
if (!match7(parser, terminator, YP_TOKEN_NEWLINE, YP_TOKEN_SEMICOLON, YP_TOKEN_EOF, YP_TOKEN_BRACE_RIGHT, YP_TOKEN_KEYWORD_DO, YP_TOKEN_PARENTHESIS_RIGHT)) {
89328945
parse_assocs(parser, (yp_node_t *) hash);
89338946
}
89348947

89358948
parsed_bare_hash = true;
8949+
parse_arguments_append(parser, arguments, argument);
89368950
break;
89378951
}
89388952
case YP_TOKEN_UAMPERSAND: {
@@ -8946,9 +8960,14 @@ parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_for
89468960
yp_diagnostic_list_append(&parser->error_list, operator.start, operator.end, YP_ERR_ARGUMENT_NO_FORWARDING_AMP);
89478961
}
89488962

8949-
argument = (yp_node_t *)yp_block_argument_node_create(parser, &operator, expression);
8963+
argument = (yp_node_t *) yp_block_argument_node_create(parser, &operator, expression);
8964+
if (parsed_block_argument) {
8965+
parse_arguments_append(parser, arguments, argument);
8966+
} else {
8967+
arguments->block = argument;
8968+
}
8969+
89508970
parsed_block_argument = true;
8951-
arguments->implicit_block = true;
89528971
break;
89538972
}
89548973
case YP_TOKEN_USTAR: {
@@ -8971,6 +8990,7 @@ parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_for
89718990
argument = (yp_node_t *) yp_splat_node_create(parser, &operator, expression);
89728991
}
89738992

8993+
parse_arguments_append(parser, arguments, argument);
89748994
break;
89758995
}
89768996
case YP_TOKEN_UDOT_DOT_DOT: {
@@ -8988,7 +9008,8 @@ parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_for
89889008
yp_diagnostic_list_append(&parser->error_list, parser->previous.start, parser->previous.end, YP_ERR_ARGUMENT_NO_FORWARDING_ELLIPSES);
89899009
}
89909010

8991-
argument = (yp_node_t *)yp_forwarding_arguments_node_create(parser, &parser->previous);
9011+
argument = (yp_node_t *) yp_forwarding_arguments_node_create(parser, &parser->previous);
9012+
parse_arguments_append(parser, arguments, argument);
89929013
break;
89939014
}
89949015
}
@@ -9031,12 +9052,11 @@ parse_arguments(yp_parser_t *parser, yp_arguments_t *arguments, bool accepts_for
90319052
parsed_bare_hash = true;
90329053
}
90339054

9055+
parse_arguments_append(parser, arguments, argument);
90349056
break;
90359057
}
90369058
}
90379059

9038-
yp_arguments_node_arguments_append(arguments->arguments, argument);
9039-
90409060
// If parsing the argument failed, we need to stop parsing arguments.
90419061
if (YP_NODE_TYPE_P(argument, YP_MISSING_NODE) || parser->recovering) break;
90429062

@@ -9736,8 +9756,6 @@ parse_arguments_list(yp_parser_t *parser, yp_arguments_t *arguments, bool accept
97369756
if (accept1(parser, YP_TOKEN_PARENTHESIS_RIGHT)) {
97379757
arguments->closing_loc = YP_LOCATION_TOKEN_VALUE(&parser->previous);
97389758
} else {
9739-
arguments->arguments = yp_arguments_node_create(parser);
9740-
97419759
yp_accepts_block_stack_push(parser, true);
97429760
parse_arguments(parser, arguments, true, YP_TOKEN_PARENTHESIS_RIGHT);
97439761
expect1(parser, YP_TOKEN_PARENTHESIS_RIGHT, YP_ERR_ARGUMENT_TERM_PAREN);
@@ -9752,7 +9770,6 @@ parse_arguments_list(yp_parser_t *parser, yp_arguments_t *arguments, bool accept
97529770
// If we get here, then the subsequent token cannot be used as an infix
97539771
// operator. In this case we assume the subsequent token is part of an
97549772
// argument to this method call.
9755-
arguments->arguments = yp_arguments_node_create(parser);
97569773
parse_arguments(parser, arguments, true, YP_TOKEN_EOF);
97579774

97589775
yp_accepts_block_stack_pop(parser);
@@ -9762,16 +9779,28 @@ parse_arguments_list(yp_parser_t *parser, yp_arguments_t *arguments, bool accept
97629779
// node that starts with a {. If there is, then we can parse it and add it to
97639780
// the arguments.
97649781
if (accepts_block) {
9782+
yp_block_node_t *block = NULL;
9783+
97659784
if (accept1(parser, YP_TOKEN_BRACE_LEFT)) {
97669785
found |= true;
9767-
arguments->block = parse_block(parser);
9786+
block = parse_block(parser);
9787+
yp_arguments_validate_block(parser, arguments, block);
97689788
} else if (yp_accepts_block_stack_p(parser) && accept1(parser, YP_TOKEN_KEYWORD_DO)) {
97699789
found |= true;
9770-
arguments->block = parse_block(parser);
9790+
block = parse_block(parser);
9791+
}
9792+
9793+
if (block != NULL) {
9794+
if (arguments->block == NULL) {
9795+
arguments->block = (yp_node_t *) block;
9796+
} else {
9797+
yp_diagnostic_list_append(&parser->error_list, block->base.location.start, block->base.location.end, YP_ERR_ARGUMENT_BLOCK_MULTI);
9798+
yp_arguments_node_arguments_append(arguments->arguments, arguments->block);
9799+
arguments->block = (yp_node_t *) block;
9800+
}
97719801
}
97729802
}
97739803

9774-
yp_arguments_validate(parser, arguments);
97759804
return found;
97769805
}
97779806

@@ -11705,7 +11734,7 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
1170511734
call->block = arguments.block;
1170611735

1170711736
if (arguments.block != NULL) {
11708-
call->base.location.end = arguments.block->base.location.end;
11737+
call->base.location.end = arguments.block->location.end;
1170911738
} else if (arguments.closing_loc.start == NULL) {
1171011739
if (arguments.arguments != NULL) {
1171111740
call->base.location.end = arguments.arguments->base.location.end;
@@ -12121,7 +12150,6 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
1212112150
yp_binding_power_t binding_power = yp_binding_powers[parser->current.type].left;
1212212151

1212312152
if (binding_power == YP_BINDING_POWER_UNSET || binding_power >= YP_BINDING_POWER_RANGE) {
12124-
arguments.arguments = yp_arguments_node_create(parser);
1212512153
parse_arguments(parser, &arguments, false, YP_TOKEN_EOF);
1212612154
}
1212712155
}
@@ -14079,11 +14107,8 @@ parse_expression_infix(yp_parser_t *parser, yp_node_t *node, yp_binding_power_t
1407914107

1408014108
if (!accept1(parser, YP_TOKEN_BRACKET_RIGHT)) {
1408114109
yp_accepts_block_stack_push(parser, true);
14082-
arguments.arguments = yp_arguments_node_create(parser);
14083-
1408414110
parse_arguments(parser, &arguments, false, YP_TOKEN_BRACKET_RIGHT);
1408514111
yp_accepts_block_stack_pop(parser);
14086-
1408714112
expect1(parser, YP_TOKEN_BRACKET_RIGHT, YP_ERR_EXPECT_RBRACKET);
1408814113
}
1408914114

@@ -14099,13 +14124,23 @@ parse_expression_infix(yp_parser_t *parser, yp_node_t *node, yp_binding_power_t
1409914124
// If we're at the end of the arguments, we can now check if there is a
1410014125
// block node that starts with a {. If there is, then we can parse it and
1410114126
// add it to the arguments.
14127+
yp_block_node_t *block = NULL;
1410214128
if (accept1(parser, YP_TOKEN_BRACE_LEFT)) {
14103-
arguments.block = parse_block(parser);
14129+
block = parse_block(parser);
14130+
yp_arguments_validate_block(parser, &arguments, block);
1410414131
} else if (yp_accepts_block_stack_p(parser) && accept1(parser, YP_TOKEN_KEYWORD_DO)) {
14105-
arguments.block = parse_block(parser);
14132+
block = parse_block(parser);
14133+
}
14134+
14135+
if (block != NULL) {
14136+
if (arguments.block != NULL) {
14137+
yp_diagnostic_list_append(&parser->error_list, block->base.location.start, block->base.location.end, YP_ERR_ARGUMENT_AFTER_BLOCK);
14138+
yp_arguments_node_arguments_append(arguments.arguments, arguments.block);
14139+
}
14140+
14141+
arguments.block = (yp_node_t *) block;
1410614142
}
1410714143

14108-
yp_arguments_validate(parser, &arguments);
1410914144
return (yp_node_t *) yp_call_node_aref_create(parser, node, &arguments);
1411014145
}
1411114146
case YP_TOKEN_KEYWORD_IN: {

‎test/yarp/errors_test.rb

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,9 @@ def test_arguments_after_block
364364
nil,
365365
Location(),
366366
Location(),
367-
ArgumentsNode([
368-
BlockArgumentNode(expression("block"), Location()),
369-
expression("foo")
370-
]),
367+
ArgumentsNode([expression("foo")]),
371368
Location(),
372-
nil,
369+
BlockArgumentNode(expression("block"), Location()),
373370
0,
374371
"a"
375372
)

‎test/yarp/location_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def test_BeginNode
6868
end
6969

7070
def test_BlockArgumentNode
71-
assert_location(BlockArgumentNode, "foo(&bar)", 4...8) { |node| node.arguments.arguments.last }
71+
assert_location(BlockArgumentNode, "foo(&bar)", 4...8, &:block)
7272
end
7373

7474
def test_BlockLocalVariableNode

0 commit comments

Comments
 (0)