Skip to content

Conversation

@nnethercote
Copy link
Contributor

A couple of improvements to things that puzzled me when I was looking at this code.

r? @matklad

A tiny bit of repetition makes this easier to read, and avoids a test on
the "Not a base prefix" match arm.
After seeing a `0`, if it's followed by any of `[0-9]`, `_`, `.`, `e`,
or `E`, we consume all the digits. But in the `.`, `e` and `E` cases
this is pointless because we know there aren't any digits.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 15, 2023
@matklad
Copy link
Contributor

matklad commented May 15, 2023

LGTM! The only thing is that, readability-wise, I see this getting refactored back down the road. Removing the branch from a common case seems good though. We could extract error construction into #cold function to emphasize that we don’t want extra branches, but it doesn’t seem to be worth it.

(although I am wondering if marking all errors as cold would be beneficial?)

@bors r+

@bors
Copy link
Collaborator

bors commented May 15, 2023

📌 Commit e52794d has been approved by matklad

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 15, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 15, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#108291 (Fix more benchmark test with black_box)
 - rust-lang#108356 (improve doc test for UnsafeCell::raw_get)
 - rust-lang#110049 (Don't claim `LocalKey::with` prevents a reference to be sent across threads)
 - rust-lang#111525 (Stop checking for the absence of something that doesn't exist)
 - rust-lang#111538 (Make sure the build.rustc version is either the same or 1 apart)
 - rust-lang#111578 (Move expansion of query macros in rustc_middle to rustc_middle::query)
 - rust-lang#111584 (Number lexing tweaks)
 - rust-lang#111587 (Custom MIR: Support `Rvalue::CopyForDeref`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 119b722 into rust-lang:master May 15, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 15, 2023
@nnethercote nnethercote deleted the number-lexing-tweaks branch May 15, 2023 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

4 participants