Skip to content

Add reset_err_count() to errors::Handler #43778

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
Merged
merged 1 commit into from
Aug 30, 2017
Merged

Add reset_err_count() to errors::Handler #43778

merged 1 commit into from
Aug 30, 2017

Conversation

@topecongiro
Copy link
Contributor

@topecongiro topecongiro commented Aug 10, 2017

The motivation here is to allow rustfmt to recover from parse errors after failing to parse macros (cc rust-lang/rustfmt#1742).
r? @nrc

@aidanhs
Copy link
Contributor

aidanhs commented Aug 10, 2017

Thanks for the PR @topecongiro! We'll check in now and again to make sure @nrc or another reviewer gets to this soon.

@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2017
@carols10cents
Copy link
Member

carols10cents commented Aug 14, 2017

friendly ping @nrc !

1 similar comment
@arielb1
Copy link
Contributor

arielb1 commented Aug 22, 2017

friendly ping @nrc !

@nrc
Copy link
Member

nrc commented Aug 22, 2017

@bors: r+

@bors
Copy link
Collaborator

bors commented Aug 22, 2017

📌 Commit a326948 has been approved by nrc

@nrc nrc 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 Aug 22, 2017
@arielb1
Copy link
Contributor

arielb1 commented Aug 22, 2017 •
edited
Loading

rustc relies on err_count being non-zero if an error happened to avoid ICEs. Could you at least add a comment that this method is dangerous @topecongiro?

@bors r-

@nrc
Copy link
Member

nrc commented Aug 22, 2017

@bors r-

nrc
nrc reviewed Aug 22, 2017
View reviewed changes
src/librustc_errors/lib.rs Outdated
Copy link
Member

@nrc nrc Aug 22, 2017

Choose a reason for hiding this comment

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

Could you add a comment to this stating that it should not be called in rustc, only by tools (it would break the compiler's error handling)

Copy link
Contributor Author

@topecongiro topecongiro Aug 23, 2017

Choose a reason for hiding this comment

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

@arielb1 @nrc Thanks for comments! I will update shortly.

@arielb1 arielb1 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 Aug 22, 2017
@topecongiro
Add reset_err_count() to errors::Handler
601e3da 
The motivation here is to allow rustfmt to recover from parse errors
after failing to parse macros.
@topecongiro topecongiro force-pushed the handler-reset-err-count branch from a326948 to 601e3da Compare August 23, 2017 12:56
@topecongiro
Copy link
Contributor Author

topecongiro commented Aug 23, 2017

Updated & rebased.

@carols10cents carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 28, 2017
@carols10cents
Copy link
Member

carols10cents commented Aug 28, 2017

Looks like this is back in your court @nrc !

@arielb1
Copy link
Contributor

arielb1 commented Aug 29, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 29, 2017

📌 Commit 601e3da has been approved by arielb1

@arielb1
Copy link
Contributor

arielb1 commented Aug 29, 2017

@bors rollup

@arielb1 arielb1 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 Aug 29, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Aug 29, 2017
Rollup merge of rust-lang#43778 - topecongiro:handler-reset-err-count…
0f2bbed 
…, r=arielb1

Add reset_err_count() to errors::Handler

The motivation here is to allow rustfmt to recover from parse errors after failing to parse macros (cc rust-lang/rustfmt#1742).
r? @nrc
@arielb1 arielb1 mentioned this pull request Aug 29, 2017
Closed
@arielb1 arielb1 mentioned this pull request Aug 29, 2017
Closed
arielb1 pushed a commit to arielb1/rust that referenced this pull request Aug 29, 2017
Rollup merge of rust-lang#43778 - topecongiro:handler-reset-err-count…
64a21f8 
…, r=arielb1

Add reset_err_count() to errors::Handler

The motivation here is to allow rustfmt to recover from parse errors after failing to parse macros (cc rust-lang/rustfmt#1742).
r? @nrc
@arielb1 arielb1 mentioned this pull request Aug 29, 2017
Merged
bors added a commit that referenced this pull request Aug 29, 2017
@bors
Auto merge of #44169 - arielb1:rollup, r=arielb1
c11f689 
Rollup of 12 pull requests

- Successful merges: #43705, #43778, #43918, #44076, #44117, #44121, #44126, #44134, #44135, #44141, #44144, #44158
- Failed merges:
@bors bors merged commit 601e3da into rust-lang:master Aug 30, 2017
@topecongiro
Copy link
Contributor Author

topecongiro commented Aug 30, 2017

Thank you!

@topecongiro topecongiro deleted the handler-reset-err-count branch August 30, 2017 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nrc nrc nrc left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@nrc nrc

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.

7 participants

@topecongiro @aidanhs @carols10cents @arielb1 @nrc @bors @alexcrichton