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

Two lexer tweaks #36921

Merged
merged 2 commits into from
Oct 4, 2016
Merged

Two lexer tweaks #36921

merged 2 commits into from
Oct 4, 2016

Conversation

nnethercote
Copy link
Contributor

19 days later, I haven't received a review of my commits in #36470. In an attempt to make some progress, I'm going to split up the changes. Here are the ones that don't relate to renaming things.

First, assert! is redundant w.r.t. the unwrap() immediately afterwards.

Second, `byte_offset_diff` is effectively computed as
`current_byte_offset + ch.len_utf8() - current_byte_offset` (with `next`
as an intermediate) which is silly and can be simplified.
@rust-highfive
Copy link
Collaborator

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

The two branches of this `if` compute the same value. This commit gets
rid of the first branch, which makes this calculation identical to the
one in scan_block_comment().
@nagisa
Copy link
Member

nagisa commented Oct 3, 2016

This seems good to me.

@arielb1 arielb1 added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 3, 2016
@nrc
Copy link
Member

nrc commented Oct 3, 2016

#36470 has been r+'ed now, so I think this PR can be closed (it will have to go through the slightly longer syntax-breaking-change landing process though). We could land this PR directly since it does not change any API, but that would mean rebasing #36470. @nnethercote do you have a preference?

@nnethercote
Copy link
Contributor Author

@nrc: At this point I'd prefer to land this and then rebase/redo #36470, because I want to change how things are done in #36470 And it's good to separate the non-breaking changes from the breaking changes.

@nrc
Copy link
Member

nrc commented Oct 3, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 3, 2016

📌 Commit 9e3dcb4 has been approved by nrc

Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 4, 2016
Two lexer tweaks

19 days later, I haven't received a review of my commits in rust-lang#36470. In an attempt to make some progress, I'm going to split up the changes. Here are the ones that don't relate to renaming things.
bors added a commit that referenced this pull request Oct 4, 2016
Rollup of 12 pull requests

- Successful merges: #36798, #36878, #36902, #36903, #36908, #36916, #36917, #36921, #36928, #36938, #36941, #36951
- Failed merges:
@bors bors merged commit 9e3dcb4 into rust-lang:master Oct 4, 2016
@nnethercote nnethercote deleted the two-lexer-tweaks branch October 4, 2016 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants