rustc: Add a warning count upon completion #69926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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
added
the
S-waiting-on-review
estebank
reviewed
Mar 11, 2020
src/librustc_errors/lib.rs
Outdated
Comment on lines
831
to
837
| self.emit_stashed_diagnostics(); | ||
| let s = match self.deduplicated_warn_count { | ||
| 0 => return, | ||
| 1 => "build completed with one warning".to_string(), | ||
| count => format!("build completed with {} warnings", count), | ||
| }; | ||
| let _ = self.emit_diagnostic(&Diagnostic::new(Level::Warning, &s)); |
There was a problem hiding this comment.
Can't this logic go inside of print_error_count?
We could even get fancy and emit aborting due to XX previous errors, YY warnings emitted.
There was a problem hiding this comment.
Agreed, that could work.
There was a problem hiding this comment.
What would be your preferred way to print it if the build succeeds, but warnings are emitted?
I was thinking about leaving it as it is (warning: build completed with x warnings or something similar).
|
Could you squash and rebase onto latest master running @bors rollup=never |
|
@bors r+ rollup=never |
|
📌 Commit 6c1f21da58bdcd1171c26e007d264ae1894eaa45 has been approved by |
bors
added
S-waiting-on-bors
|
CC @rust-lang/compiler adding a count for warnings |
varkor
reviewed
Mar 27, 2020
src/librustc_errors/lib.rs
Outdated
| 0 => return, | ||
| let warnings = match self.deduplicated_warn_count { | ||
| 0 => String::new(), | ||
| 1 => "one warning emitted".to_string(), |
There was a problem hiding this comment.
Suggested change
| 1 => "one warning emitted".to_string(), | |
| 1 => "1 warning emitted".to_string(), |
| @@ -15,6 +15,6 @@ LL | #![plugin(empty_plugin)] | |||
| | | |||
| = note: `#[warn(deprecated)]` on by default | |||
|
|
|||
| error: aborting due to previous error | |||
| error: aborting due to previous error, one warning emitted | |||
There was a problem hiding this comment.
It seems better just to stick with the usual warning, rather than merge it into the error message. The compilation isn't aborting due to the warning at all.
There was a problem hiding this comment.
True, but that unnecessary wastes vertical space, which I. keep hearing complaints about.
There was a problem hiding this comment.
Maybe we could have error: aborting due to previous error (1 warning emitted) or error: aborting due to previous error; 1 warning emitted instead, then. A comma seems inappropriate here.
bors
added
S-waiting-on-author
|
Sure, what about the change @varkor was talking about regarding errors? |
|
The |
|
Waiting on the build. I've also rebased everything onto the most recent changes to master. |
|
Could you also squash the stderr changes together? (So that there is either one, or two commits: the code change, and the stderr changes.) This helps keep the git commit cleaner, which is convenient when using |
|
📌 Commit 9f594d2bb0c11d7515dfcf34f7647d041594fa51 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 9f594d2bb0c11d7515dfcf34f7647d041594fa51 with merge d8564429444a927c3d361f296908dec8e34d3d55... |
|
💔 Test failed - checks-azure |
|
@bors p=1 |
|
⌛ Testing commit 2d8a8e2 with merge ba1f7570741b6069079a6e059bf395963fdbb119... |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
Failure: I tried to |
|
@roccodev: ah, I think you need to include the |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Some tests were introduced today/yesterday; |
|
@bors r=estebank,varkor rollup=never |
|
📌 Commit b85c64c has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
rust-highfive
added a commit
to rust-lang-nursery/rust-toolstate
that referenced
this pull request
Apr 12, 2020
Tested on commit rust-lang/rust@941d435. Direct link to PR: <rust-lang/rust#69926> 💔 rls on linux: test-pass → test-fail (cc @Xanewok).
bors
added a commit
to rust-lang/cargo
that referenced
this pull request
Apr 13, 2020
Fix nightly test matching rustc "warning" output. rust-lang/rust#69926 changed the warning output from rustc. #8080 attempted to compensate for it, but missed one of the cases. I don't think this test needs to be quite so exhaustive about checking the output.
Mark-Simulacrum
pushed a commit
to rust-lang/cargo
that referenced
this pull request
May 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a
build completed with one warning/x warningsmessage, similar to the already presentaborted due to previous errormessage.