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

parse_tt cleanups #94547

Merged
merged 5 commits into from
Mar 4, 2022
Merged

parse_tt cleanups #94547

merged 5 commits into from
Mar 4, 2022

Conversation

nnethercote
Copy link
Contributor

I've been looking closely at this code, and saw some opportunities to improve its readability.

r? @petrochenkov

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 3, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 3, 2022
@nnethercote
Copy link
Contributor Author

@bors rollup=always

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

r=me after updating the formatting.

@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2022
@nnethercote
Copy link
Contributor Author

I have retriggered the tests. There were no formatting problems locally, and the output of the failure didn't match the file contents... not sure what happened.

To avoid the strange style where comments force `else` onto its own
line.

The commit also removes several else-after-return constructs, which can
be hard to read.
There are three `Option` fields in `MatcherPos` that are only used in
tandem. This commit combines them, making the code slightly easier to
read. (It also makes clear that the `sep` field arguably should have
been `Option<Option<Token>>`!)
Because `inner_parse_loop` has only one way to not succeed, not three.
@nnethercote
Copy link
Contributor Author

nnethercote commented Mar 3, 2022

Ah... I've occasionally had problems where a particular file doesn't get formatted by x.py fmt, for no clear reason. I just worked out why. I had a copy of compiler/rustc_expand/src/mbe/macro_parser.rs in the root of my clone. Because it's not under version control x.py fmt ignores it... and somehow this ignoring extends to compiler/rustc_expand/src/mbe/macro_parser.rs as well. When I renamed that copy to something else the formatting worked again.

@nnethercote
Copy link
Contributor Author

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Mar 3, 2022

📌 Commit 97eb1b4 has been approved by petrochenkov

@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 Mar 3, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 3, 2022
…trochenkov

`parse_tt` cleanups

I've been looking closely at this code, and saw some opportunities to improve its readability.

r? `@petrochenkov`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 3, 2022
…trochenkov

`parse_tt` cleanups

I've been looking closely at this code, and saw some opportunities to improve its readability.

r? `````@petrochenkov`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 3, 2022
…trochenkov

`parse_tt` cleanups

I've been looking closely at this code, and saw some opportunities to improve its readability.

r? ``````@petrochenkov``````
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#92697 (Use cgroup quotas for calculating `available_parallelism`)
 - rust-lang#94057 (improve comments for `simplify_type`)
 - rust-lang#94547 (`parse_tt` cleanups)
 - rust-lang#94550 (rustdoc: Add test for higher kinded functions generated by macros)
 - rust-lang#94551 (Doc: Fix use of quote instead of backstick in Adapter::map.)
 - rust-lang#94554 (Fix invalid lint_node_id being put on a removed stmt)
 - rust-lang#94555 (all: fix some typos)
 - rust-lang#94563 (Remove a unnecessary `..` pattern)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 16c6594 into rust-lang:master Mar 4, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 4, 2022
@nnethercote nnethercote deleted the parse_tt-cleanups branch March 4, 2022 00:48
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.

6 participants