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.

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

Remove forward declarations of local variables #2132

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

amomchilov
Copy link
Contributor

Our work to migrate the parser off its dependency on Ruby involves introducing new local variables, and incrementally changing types (e.g one branch if an if might use a VALUE, while another uses a new rbs_node_t).

To minimize the diff, I've extracted these 2 commits. While long, they're making two simple changes:

  1. Add { } blocks around certain case bodies, in which our code will need to introduce new local variables.
  2. Move the forward-declarations of local variables down to be as close to their point of initialization, as possible.

@amomchilov amomchilov force-pushed the remove-fowards-decls branch 2 times, most recently from 3033039 to 50aae04 Compare December 6, 2024 20:43
@@ -2731,8 +2706,6 @@ static void parse_use_clauses(parserstate *state, VALUE clauses) {
range namespace_range = NULL_RANGE;
VALUE namespace = parse_namespace(state, &namespace_range);

range clause_range = namespace_range;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value is only used different in the two blocks below, so I split it into two local variables.

@amomchilov amomchilov mentioned this pull request Dec 6, 2024
7 tasks
@amomchilov amomchilov force-pushed the remove-fowards-decls branch from 50aae04 to f8adfee Compare December 6, 2024 21:08
@amomchilov amomchilov changed the title Remove forward declarations of local variables, where possible Remove forward declarations of local variables Dec 6, 2024
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@soutaro soutaro added this to the RBS 3.8 milestone Dec 9, 2024
@soutaro soutaro added this pull request to the merge queue Dec 9, 2024
Merged via the queue into ruby:master with commit 514eafa Dec 9, 2024
18 checks passed
@soutaro soutaro added the Released PRs already included in the released version label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants