Skip to content

Add a "repeated flag" to parameter nodes #2144

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

Merged
kddnewton merged 2 commits into from
Jan 10, 2024
Merged

Add a "repeated flag" to parameter nodes #2144

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
b443cb1
Add a "repeated flag" to parameter nodes
tenderlove Jan 9, 2024
ed183ad
address feedback
tenderlove Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ flags:
- name: BEGIN_MODIFIER
comment: "a loop after a begin statement, so the body is executed first before the condition"
comment: Flags for while and until loop nodes.
- name: ParameterFlags
values:
- name: REPEATED_PARAMETER
comment: "a parameter name that has been repeated in the method signature"
comment: Flags for parameter nodes.
- name: RangeFlags
values:
- name: EXCLUDE_END
Expand Down Expand Up @@ -648,6 +653,9 @@ nodes:
^^^^^^^^^^
- name: BlockLocalVariableNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant
comment: |
Expand Down Expand Up @@ -676,6 +684,9 @@ nodes:
^^^^^^^^^^^^^^
- name: BlockParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant?
- name: name_loc
Expand Down Expand Up @@ -1923,6 +1934,9 @@ nodes:
^^^^
- name: KeywordRestParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant?
- name: name_loc
Expand Down Expand Up @@ -2221,6 +2235,9 @@ nodes:
^^
- name: OptionalKeywordParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant
- name: name_loc
Expand All @@ -2235,6 +2252,9 @@ nodes:
end
- name: OptionalParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant
- name: name_loc
Expand Down Expand Up @@ -2451,6 +2471,9 @@ nodes:
^^^^^^
- name: RequiredKeywordParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant
- name: name_loc
Expand All @@ -2463,6 +2486,9 @@ nodes:
end
- name: RequiredParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant
comment: |
Expand Down Expand Up @@ -2514,6 +2540,9 @@ nodes:
`ex` is in the `exception` field.
- name: RestParameterNode
fields:
- name: flags
type: flags
kind: ParameterFlags
- name: name
type: constant?
- name: name_loc
Expand Down
12 changes: 8 additions & 4 deletions lib/prism/debug.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,28 +138,32 @@ def self.prism_locals(source)
*params.keywords.grep(OptionalKeywordParameterNode).map(&:name),
]

sorted << AnonymousLocal if params.keywords.any?

if params.keyword_rest.is_a?(ForwardingParameterNode)
sorted.push(:*, :&, :"...")
elsif params.keyword_rest.is_a?(KeywordRestParameterNode)
sorted << params.keyword_rest.name if params.keyword_rest.name
end

sorted << AnonymousLocal if params.keywords.any?

# Recurse down the parameter tree to find any destructured
# parameters and add them after the other parameters.
param_stack = params.requireds.concat(params.posts).grep(MultiTargetNode).reverse
while (param = param_stack.pop)
case param
when MultiTargetNode
param_stack.concat(param.rights.reverse)
param_stack << param.rest
param_stack << param.rest if param.rest&.expression && !sorted.include?(param.rest.expression.name)
param_stack.concat(param.lefts.reverse)
when RequiredParameterNode
sorted << param.name
when SplatNode
sorted << param.expression.name if param.expression
sorted << param.expression.name
end
end

sorted << params.block.name if params.block&.name

names = sorted.concat(names - sorted)
end

Expand Down
85 changes: 71 additions & 14 deletions src/prism.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,22 @@ pm_node_flag_unset(pm_node_t *node, pm_node_flags_t flag) {
node->flags &= (pm_node_flags_t) ~flag;
}

/**
* Set the repeated parameter flag on the given node.
*/
static inline void
pm_node_flag_set_repeated_parameter(pm_node_t *node) {
assert(PM_NODE_TYPE(node) == PM_BLOCK_LOCAL_VARIABLE_NODE ||
PM_NODE_TYPE(node) == PM_BLOCK_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_KEYWORD_REST_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_OPTIONAL_KEYWORD_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_OPTIONAL_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_REQUIRED_KEYWORD_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_REQUIRED_PARAMETER_NODE ||
PM_NODE_TYPE(node) == PM_REST_PARAMETER_NODE);

pm_node_flag_set(node, PM_PARAMETER_FLAGS_REPEATED_PARAMETER);
}

/******************************************************************************/
/* Node creation functions */
Expand Down Expand Up @@ -5995,23 +6011,28 @@ pm_parser_local_add_owned(pm_parser_t *parser, const uint8_t *start, size_t leng
/**
* Add a parameter name to the current scope and check whether the name of the
* parameter is unique or not.
*
* Returns `true` if this is a duplicate parameter name, otherwise returns
* false.
*/
static void
static bool
Copy link
Copy Markdown
Collaborator

@kddnewton kddnewton Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you document the return type in the description of the function?

pm_parser_parameter_name_check(pm_parser_t *parser, const pm_token_t *name) {
// We want to check whether the parameter name is a numbered parameter or
// not.
pm_refute_numbered_parameter(parser, name->start, name->end);

// We want to ignore any parameter name that starts with an underscore.
if ((name->start < name->end) && (*name->start == '_')) return;

// Otherwise we'll fetch the constant id for the parameter name and check
// whether it's already in the current scope.
pm_constant_id_t constant_id = pm_parser_constant_id_token(parser, name);

if (pm_constant_id_list_includes(&parser->current_scope->locals, constant_id)) {
pm_parser_err_token(parser, name, PM_ERR_PARAMETER_NAME_REPEAT);
// Add an error if the parameter doesn't start with _ and has been seen before
if ((name->start < name->end) && (*name->start != '_')) {
pm_parser_err_token(parser, name, PM_ERR_PARAMETER_NAME_REPEAT);
}
return true;
}
return false;
}

/**
Expand Down Expand Up @@ -11448,7 +11469,9 @@ parse_required_destructured_parameter(pm_parser_t *parser) {
if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
pm_token_t name = parser->previous;
value = (pm_node_t *) pm_required_parameter_node_create(parser, &name);
pm_parser_parameter_name_check(parser, &name);
if (pm_parser_parameter_name_check(parser, &name)) {
pm_node_flag_set_repeated_parameter(value);
}
pm_parser_local_add_token(parser, &name);
}

Expand All @@ -11458,7 +11481,9 @@ parse_required_destructured_parameter(pm_parser_t *parser) {
pm_token_t name = parser->previous;

param = (pm_node_t *) pm_required_parameter_node_create(parser, &name);
pm_parser_parameter_name_check(parser, &name);
if (pm_parser_parameter_name_check(parser, &name)) {
pm_node_flag_set_repeated_parameter(param);
}
pm_parser_local_add_token(parser, &name);
}

Expand Down Expand Up @@ -11575,9 +11600,10 @@ parse_parameters(
pm_token_t operator = parser->previous;
pm_token_t name;

bool repeated = false;
if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
name = parser->previous;
pm_parser_parameter_name_check(parser, &name);
repeated = pm_parser_parameter_name_check(parser, &name);
pm_parser_local_add_token(parser, &name);
} else {
name = not_provided(parser);
Expand All @@ -11588,6 +11614,9 @@ parse_parameters(
}

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);
}
if (params->block == NULL) {
pm_parameters_node_block_set(params, param);
} else {
Expand Down Expand Up @@ -11660,7 +11689,7 @@ parse_parameters(
}

pm_token_t name = parser->previous;
pm_parser_parameter_name_check(parser, &name);
bool repeated = pm_parser_parameter_name_check(parser, &name);
pm_parser_local_add_token(parser, &name);

if (accept1(parser, PM_TOKEN_EQUAL)) {
Expand All @@ -11671,6 +11700,9 @@ parse_parameters(
pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT);

pm_optional_parameter_node_t *param = pm_optional_parameter_node_create(parser, &name, &operator, value);
if (repeated) {
pm_node_flag_set_repeated_parameter((pm_node_t *)param);
}
pm_parameters_node_optionals_append(params, param);

parser->current_param_name = old_param_name;
Expand All @@ -11685,9 +11717,15 @@ parse_parameters(
}
} else if (order > PM_PARAMETERS_ORDER_AFTER_OPTIONAL) {
pm_required_parameter_node_t *param = pm_required_parameter_node_create(parser, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter((pm_node_t *)param);
}
pm_parameters_node_requireds_append(params, (pm_node_t *) param);
} else {
pm_required_parameter_node_t *param = pm_required_parameter_node_create(parser, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter((pm_node_t *)param);
}
pm_parameters_node_posts_append(params, (pm_node_t *) param);
}

Expand All @@ -11702,14 +11740,17 @@ parse_parameters(
pm_token_t local = name;
local.end -= 1;

pm_parser_parameter_name_check(parser, &local);
bool repeated = pm_parser_parameter_name_check(parser, &local);
pm_parser_local_add_token(parser, &local);

switch (parser->current.type) {
case PM_TOKEN_COMMA:
case PM_TOKEN_PARENTHESIS_RIGHT:
case PM_TOKEN_PIPE: {
pm_node_t *param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter(param);
}
pm_parameters_node_keywords_append(params, param);
break;
}
Expand All @@ -11721,6 +11762,9 @@ parse_parameters(
}

pm_node_t *param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter(param);
}
pm_parameters_node_keywords_append(params, param);
break;
}
Expand All @@ -11740,6 +11784,9 @@ parse_parameters(
param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
}

if (repeated) {
pm_node_flag_set_repeated_parameter(param);
}
pm_parameters_node_keywords_append(params, param);

// If parsing the value of the parameter resulted in error recovery,
Expand All @@ -11762,10 +11809,10 @@ parse_parameters(

pm_token_t operator = parser->previous;
pm_token_t name;

bool repeated = false;
if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
name = parser->previous;
pm_parser_parameter_name_check(parser, &name);
repeated = pm_parser_parameter_name_check(parser, &name);
pm_parser_local_add_token(parser, &name);
} else {
name = not_provided(parser);
Expand All @@ -11776,6 +11823,9 @@ parse_parameters(
}

pm_node_t *param = (pm_node_t *) pm_rest_parameter_node_create(parser, &operator, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter(param);
}
if (params->rest == NULL) {
pm_parameters_node_rest_set(params, param);
} else {
Expand All @@ -11798,9 +11848,10 @@ parse_parameters(
} else {
pm_token_t name;

bool repeated = false;
if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
name = parser->previous;
pm_parser_parameter_name_check(parser, &name);
repeated = pm_parser_parameter_name_check(parser, &name);
pm_parser_local_add_token(parser, &name);
} else {
name = not_provided(parser);
Expand All @@ -11811,6 +11862,9 @@ parse_parameters(
}

param = (pm_node_t *) pm_keyword_rest_parameter_node_create(parser, &operator, &name);
if (repeated) {
pm_node_flag_set_repeated_parameter(param);
}
}

if (params->keyword_rest == NULL) {
Expand Down Expand Up @@ -12046,10 +12100,13 @@ parse_block_parameters(
if ((opening->type != PM_TOKEN_NOT_PROVIDED) && accept1(parser, PM_TOKEN_SEMICOLON)) {
do {
expect1(parser, PM_TOKEN_IDENTIFIER, PM_ERR_BLOCK_PARAM_LOCAL_VARIABLE);
pm_parser_parameter_name_check(parser, &parser->previous);
bool repeated = pm_parser_parameter_name_check(parser, &parser->previous);
pm_parser_local_add_token(parser, &parser->previous);

pm_block_local_variable_node_t *local = pm_block_local_variable_node_create(parser, &parser->previous);
if (repeated) {
pm_node_flag_set_repeated_parameter((pm_node_t *)local);
}
pm_block_parameters_node_append_local(block_parameters, local);
} while (accept1(parser, PM_TOKEN_COMMA));
}
Expand Down
Loading