Skip to content

Move require_eof handling to C API#2678

Merged
soutaro merged 2 commits intomasterfrom
fix-eol-type
Nov 3, 2025
Merged

Move require_eof handling to C API#2678
soutaro merged 2 commits intomasterfrom
fix-eol-type

Conversation

@soutaro
Copy link
Member

@soutaro soutaro commented Oct 8, 2025

The require_eof: argument in Parser.parse_method_type is handled by the Ruby extension layer, but this PR moves the feature to the C API, enabling other RBS parser clients, like Sorbet, to utilize it as well.

@soutaro soutaro added this to the RBS 4.0 milestone Oct 8, 2025
src/parser.c Outdated
}
}

CHECK_PARSE(parser_pop_typevar_table(parser));
Copy link
Member

Choose a reason for hiding this comment

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

Can you try moving this before the if condition above?

  CHECK_PARSE(parser_pop_typevar_table(parser));

  if (require_eof) {
      rbs_parser_advance(parser);
      if (parser->current_token.type != pEOF) {
          rbs_parser_set_error(parser, parser->current_token, true, "expected a token `%s`", rbs_token_type_str(pEOF));
          return false;
      }
  }

Copy link
Member

Choose a reason for hiding this comment

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

This makes sure when we see pEOF, the typevar table is still popped.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... Confirmed the typevar_table is allocated through the allocator. So, this should not actually cause a leak. 🤔

@soutaro soutaro added this pull request to the merge queue Nov 3, 2025
Merged via the queue into master with commit d86ab02 Nov 3, 2025
21 of 22 checks passed
@soutaro soutaro deleted the fix-eol-type branch November 3, 2025 05:37
@soutaro soutaro added the to-backport:3.10 PRs to be backported to 3.10 label Dec 17, 2025
soutaro added a commit that referenced this pull request Dec 17, 2025
Move `require_eof` handling to C API
@soutaro soutaro removed the to-backport:3.10 PRs to be backported to 3.10 label Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants