Improve char_lit's readability and speed #36414
Conversation
This makes the function more concise and easier to understand.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| let idx = match lit.find('}') { | ||
| Some(idx2) => idx2, | ||
| None => panic!("lexer should have rejected a bad character escape {}", lit), | ||
| }; |
There was a problem hiding this comment.
I find this format more verbose and adds no new information.
There was a problem hiding this comment.
That's true, but you're overlooking the speed improvement which is the motivation for the change.
There was a problem hiding this comment.
You could make this a bit less verbose while still keeping the speed improvement with unwrap_or_else:
let idx = lit.find('}').unwrap_or_else(|| {
panic!("lexer should have rejected a bad character escape {}", lit)
});
|
@nnethercote Thanks for the PR! |
|
@jseyfried: I'll try that. Should I modify the second patch, or make the change in an additional patch? |
|
@nnethercote I would just amend the second commit. |
This reduces the time taken to run `rustc -Zparse-only rustc-benchmarks/issue-32278-big-array-of-strings` from 0.18s to 0.15s on my machine, and reduces the number of instructions (as measured by Cachegrind) from 1.34B to 1.01B. With the change applied, the time to fully compile that benchmark is 1.96s, so this is a 1.5% improvement.
nnethercote
force-pushed
the
char_lit
branch
from
5b1c782 to
826f673
Compare
|
I updated the second commit. r? @jseyfried |
|
@bors r+ |
|
📌 Commit 826f673 has been approved by |
Improve char_lit's readability and speed This is my first contribution to rustc. Please let me know if I've done anything wrong. (I ran `make tidy` before making the pull request.)
This is my first contribution to rustc. Please let me know if I've done anything wrong. (I ran
make tidybefore making the pull request.)