Clarify the positions of the lexer and parser#36969
Merged
bors merged 6 commits intorust-lang:masterfrom Oct 18, 2016
Merged
Conversation
This is a [breaking-change] for libsyntax.
Likewise, rename LastTokenKind as PrevTokenKind. This is a [breaking-change] for libsyntax.
This is a [breaking-change] for libsyntax.
This is a [breaking-change] for libsyntax.
Likewise, rename StringReader::curr_is as ch_is. This is a [breaking-change] for libsyntax.
This commit renames the variables to make it clearer which char each one refers to. It also slightly reorders and rearranges some statements.
Contributor
|
(rust_highfive has picked a reviewer for you, use r? to override) |
Contributor
Author
Contributor
|
r? @eddyb |
Contributor
|
To clarify, we're blocked here on @Manishearth doing some kind of "rollup of plugin-breaking changes"? /me can't wait for macros 1.1 |
Member
Collaborator
|
📌 Commit 94b3659 has been approved by |
Member
|
Yeah libsyntax won't affect Serde anymore. I still appreciate the heads-up because aster and quasi still exist. |
Contributor
Yay. This is a useful patch and it'd be nice to have it merged sooner rather than later. |
bors
added a commit
that referenced
this pull request
Oct 18, 2016
Clarify the positions of the lexer and parser The lexer and parser use unclear names to indicate their positions in the source code. I propose the following renamings. Lexer: ``` pos -> next_pos # it's actually the next pos! last_pos -> pos # it's actually the current pos! curr -> ch # the current char curr_is -> ch_is # tests the current char col (unchanged) # the current column ``` parser ``` - last_span -> prev_span # the previous token's span - last_token_kind -> prev_token_kind # the previous token's kind - LastTokenKind -> PrevTokenKind # ditto (but the type) - token (unchanged) # the current token - span (unchanged) # the current span ``` Things to note: - This proposal removes all uses of "last", which is an unclear word because it could mean (a) previous, (b) final, or (c) most recent, i.e. current. - The "current" things (ch, col, token, span) consistently lack a prefix. The "previous" and "next" things consistently have a prefix.
Collaborator
|
⌛ Testing commit 94b3659 with merge 3543a0f... |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The lexer and parser use unclear names to indicate their positions in the
source code. I propose the following renamings.
Lexer:
parser
Things to note:
could mean (a) previous, (b) final, or (c) most recent, i.e. current.
"previous" and "next" things consistently have a prefix.