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

Collect occurrences of empty blocks for mismatched braces diagnostic #65364

Merged
merged 3 commits into from
Oct 19, 2019
Merged

Collect occurrences of empty blocks for mismatched braces diagnostic #65364

merged 3 commits into from
Oct 19, 2019

Conversation

XiangQingW
Copy link
Contributor

@XiangQingW XiangQingW commented Oct 13, 2019 •
edited by estebank
Loading

Fix #63904

@XiangQingW
Collect occurrences of for mismatched braces diagnostic
e6ce3ef 
Change-Id: I20ba0b62308370ee961141fa1aefc4b9c9f0cb3a
@rust-highfive
Copy link
Collaborator

r? @eddyb

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

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

This comment has been minimized.

Sign in to view
@XiangQingW
Collect occurrences of for mismatched braces diagnostic
888d0ba 
Change-Id: I20ba0b62308370ee961141fa1aefc4b9c9f0cb3a
@XiangQingW
Copy link
Contributor Author

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned eddyb Oct 13, 2019
@XiangQingW
Copy link
Contributor Author

@estebank Would you mind reviewing this pr if you have free time.

estebank
estebank reviewed Oct 14, 2019
View reviewed changes
src/libsyntax/parse/lexer/tokentrees.rs Outdated
@@ -121,13 +124,20 @@ impl<'a> TokenTreesReader<'a> {
// Correct delimiter.
token::CloseDelim(d) if d == delim => {
let (open_brace, open_brace_span) = self.open_braces.pop().unwrap();
let close_brace_span = self.token.span;

if close_brace_span.lo() == open_brace_span.hi() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that it would make more sense to check whether tts is empty instead of comparing spans. That way we also catch cases like { } and {\n}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the advice and I'll to change it.

@estebank
Copy link
Contributor

@XiangQingW love it! There's one minor change I'd like, but otherwise r=me.

@estebank estebank changed the title Collect occurrences of for mismatched braces diagnostic Collect occurrences of empty blocks for mismatched braces diagnostic Oct 15, 2019
@XiangQingW
Collect occurrences of for mismatched braces diagnostic
fe819a0 
Change-Id: I20ba0b62308370ee961141fa1aefc4b9c9f0cb3a
@estebank
Copy link
Contributor

@bors +r rollup

@tesuji
Copy link
Contributor

tesuji commented Oct 16, 2019

r+ ?

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 16, 2019

📌 Commit fe819a0 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 16, 2019
@XiangQingW
Copy link
Contributor Author

XiangQingW commented Oct 17, 2019 •
edited
Loading

@ebarnard Pinging again from triage. I have reverted the unrelated commit. Thank you.

@tesuji
Copy link
Contributor

tesuji commented Oct 17, 2019

Please remove it entirely by rebasing.

@ebarnard
Copy link
Contributor

@XiangQingW I think you mean @estebank.

@estebank
Copy link
Contributor

@XiangQingW can you use git rebase -i as requested by @lzutao to remove the last two commits from this branch and then git push --force?

Double check the log before and after the rebase before pushing to make sure what you expect to be there is there.

@XiangQingW
Copy link
Contributor Author

@XiangQingW can you use git rebase -i as requested by @lzutao to remove the last two commits from this branch and then git push --force?

Double check the log before and after the rebase before pushing to make sure what you expect to be there is there.

It's really helpful. Thank you very much.

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 18, 2019

📌 Commit fe819a0 has been approved by estebank

tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
@tmandry
Rollup merge of rust-lang#65364 - XiangQingW:master, r=estebank
c3eaee3 
Collect occurrences of empty blocks for mismatched braces diagnostic

Fix rust-lang#63904
@tmandry tmandry mentioned this pull request Oct 18, 2019
Closed
bors added a commit that referenced this pull request Oct 18, 2019
@bors
Auto merge of #65565 - tmandry:rollup-s8n9bie, r=tmandry
3623bcc 
Rollup of 17 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65417 (Add more coherence tests)
 - #65434 (Add long error explanation for E0577)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65471 (Add long error explanation for E0578)

r? @ghost
tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
@tmandry
Rollup merge of rust-lang#65364 - XiangQingW:master, r=estebank
8f8a23f 
Collect occurrences of empty blocks for mismatched braces diagnostic

Fix rust-lang#63904
@tmandry tmandry mentioned this pull request Oct 18, 2019
Merged
bors added a commit that referenced this pull request Oct 19, 2019
@bors
Auto merge of #65570 - tmandry:rollup-hck39pf, r=tmandry
14f0ed6 
Rollup of 19 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65334 (Add long error explanation for E0575)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65493 (Add long error explanation for E0584)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65498 (Plugins deprecation: don’t suggest simply removing the attribute)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65535 (rustc: arena-allocate the slice in `ty::GenericsPredicate`, not the whole struct.)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65390 (Add long error explanation for E0576)
 - #65434 (Add long error explanation for E0577)
 - #65471 (Add long error explanation for E0578)

r? @ghost
@bors bors merged commit fe819a0 into rust-lang:master Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

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.

Collect occurrences of {} for mismatched braces diagnostic
7 participants
@XiangQingW @rust-highfive @estebank @tesuji @bors @ebarnard @eddyb