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.

Sign up for GitHub

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

Jump to bottom

Improve code clarity #64935

Merged
merged 23 commits into from
Oct 1, 2019
Merged

Improve code clarity #64935

merged 23 commits into from
Oct 1, 2019

Conversation

AnthonyMikh
Copy link
Contributor

No commit except 55b5428 address performance, just making the existing code more clear.

r? @estebank

@AnthonyMikh
Use saturating_sub where applicable
2cb460e
@AnthonyMikh
Use max instead of ifs
e9a93be
@AnthonyMikh
Simplify Unicode-aware trimming
4fc5650
@AnthonyMikh
Use map + sum instead of fold for computing Unicode width
d6327e8
@AnthonyMikh
Use Option::map_or where applicable
aef169b
@AnthonyMikh
Unify order of variables in chained comparison
9b447e2
@AnthonyMikh
Simplify Emitter::fix_multispan_in_std_macros
f93827f 
1. Rewrite `if let` into `match` to return earl and avoid indenting giant block 
2. Assign `spans_updated` only once
@AnthonyMikh
Use sort_by_key rather than sort_by
ea32862
@AnthonyMikh
Simplify style_or_override
7a0725f
@AnthonyMikh
Revert "Simplify Unicode-aware trimming"
75a7c27 
`taken` is actually used afterwards
@AnthonyMikh
Simplify code for special case of annotation
7dc953b
@AnthonyMikh
Simplify EmitterWriter::get_multispan_max_line_num
6b6a79b
@AnthonyMikh
Simplify EmitterWriter::get_max_line_num
7b4c5c6
@AnthonyMikh
Update doc comment for style_or_override
21724ed
@AnthonyMikh
Simplify EmitterWriter::emit_suggestion_default
7a807c5 
Make function return early if source map is not present
@AnthonyMikh
Remove redundant .iter_mut()
9d73176
@AnthonyMikh
Simplify a conditional in collect_annotations
55b5428 
Also avoid excessive cloning
@AnthonyMikh
Use pattern matching instead of indexing tuples
bb44288
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
@AnthonyMikh
Copy link
Contributor Author

TIL that override is a Rust keyword

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@AnthonyMikh
Fix borrowck errors
50c2a58 
Reborrowing doesn't work for loops
@estebank
Copy link
Contributor

estebank commented Oct 1, 2019

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 1, 2019

📌 Commit 50c2a58 has been approved by estebank

@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 Oct 1, 2019
@estebank
Copy link
Contributor

estebank commented Oct 1, 2019

@bors r-

I'll approve once CI is green.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 1, 2019
@AnthonyMikh
Copy link
Contributor Author

AnthonyMikh commented Oct 1, 2019 •
edited
Loading

Should compiling rustc_plugin take so long? CI seems to stuck on it

UPD: Yes, it takes that long

@Centril
Copy link
Contributor

Centril commented Oct 1, 2019

CI is green now. @bors r=estebank

@bors
Copy link
Contributor

bors commented Oct 1, 2019

📌 Commit 50c2a58 has been approved by estebank

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 1, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
@Centril
Rollup merge of rust-lang#64935 - AnthonyMikh:librustc_errors/emmiter…
3ff0f33 
…__code-clarity, r=estebank

Improve code clarity

No commit except 55b5428 address performance, just making the existing code more clear.

r? @estebank
@Centril Centril mentioned this pull request Oct 1, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
@Centril
Rollup merge of rust-lang#64935 - AnthonyMikh:librustc_errors/emmiter…
5c6a8ee 
…__code-clarity, r=estebank

Improve code clarity

No commit except 55b5428 address performance, just making the existing code more clear.

r? @estebank
@Centril Centril mentioned this pull request Oct 1, 2019
Merged
bors added a commit that referenced this pull request Oct 1, 2019
@bors
Auto merge of #64946 - Centril:rollup-66mj5o0, r=Centril
702b45e 
Rollup of 10 pull requests

Successful merges:

 - #63674 (syntax: Support modern attribute syntax in the `meta` matcher)
 - #63931 (Stabilize macros in some more positions)
 - #64887 (syntax: recover trailing `|` in or-patterns)
 - #64895 (async/await: improve not-send errors)
 - #64896 (Remove legacy grammar)
 - #64907 (A small amount of tidying-up factored out from PR #64648)
 - #64928 (Add tests for some issues)
 - #64930 (Silence unreachable code lint from await desugaring)
 - #64935 (Improve code clarity)
 - #64937 (Deduplicate closure type errors)

Failed merges:

r? @ghost
@bors bors merged commit 50c2a58 into rust-lang:master Oct 1, 2019
@dtolnay dtolnay mentioned this pull request Oct 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AnthonyMikh @rust-highfive @estebank @bors @Centril