Skip to content

Return a plain Token for Prism.lex etc. - #4206

Open
Earlopain wants to merge 1 commit into
ruby:mainfrom
Earlopain:lex-state-internal
Open

Return a plain Token for Prism.lex etc.#4206
Earlopain wants to merge 1 commit into
ruby:mainfrom
Earlopain:lex-state-internal

Conversation

@Earlopain

@Earlopain Earlopain commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Instead of a 2-element array. Nobody should care about the state, and nobody seems to. A code search on Github shows that it only gets discarded (like via value.map(&:first), or similar things.

But because our own ripper translator still needs it for compatibility, we have to keep it around in some way. I played around with something like Prism.lex_internal that keeps the same return value but that duplicates much logic like serialization/deserialization.

This achieves the same thing and is much smaller in scope.

Comment thread ext/prism/extension.c
@@ -846,14 +846,7 @@ parse_lex_token(pm_parser_t *parser, pm_token_t *token, void *data) {
parse_lex_data_t *parse_lex_data = (parse_lex_data_t *) data;

VALUE value = pm_token_new(parser, token, parse_lex_data->encoding, parse_lex_data->source, parse_lex_data->freeze);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This already freezes the returned token if applicable, not necessary here again

Instead of a 2-element array. Nobody should care about the state, and nobody seems to.
A code search on Github shows that it only gets discarded (like via `value.map(&:first)`, or similar things.

But because our own ripper translator still needs it for compatibility, we have to keep it around in some way.
I played around with `Prism.lex_internal` that keeps the same return value but that duplicates much
logic like serialization/deserialization.

This achieves the same thing and is much smaller in scope.
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.

1 participant