Merged
Conversation
Contributor
|
Looks good to me on a first pass, but given that this touches r? @oli-obk |
Contributor
Contributor
|
The other changes seem fine to me. I will be refactoring them in the future anyway, so cleaning up the API for them a bit just makes my job easier ^^ |
Collaborator
|
☔ The latest upstream changes (presumably #120112) made this pull request unmergeable. Please resolve the merge conflicts. |
We have several methods indicating the presence of errors, lint errors, and delayed bugs. I find it frustrating that it's very unclear which one you should use in any particular spot. This commit attempts to instill a basic principle of "use the least general one possible", because that reflects reality in practice -- `has_errors` is the least general one and has by far the most uses (esp. via `abort_if_errors`). Specifics: - Add some comments giving some usage guidelines. - Prefer `has_errors` to comparing `err_count` to zero. - Remove `has_errors_or_span_delayed_bugs` because it's a weird one: in the cases where we need to count delayed bugs, we should really be counting lint errors as well. - Rename `is_compilation_going_to_fail` as `has_errors_or_lint_errors_or_span_delayed_bugs`, for consistency with `has_errors` and `has_errors_or_lint_errors`. - Change a few other `has_errors_or_lint_errors` calls to `has_errors`, as per the "least general" principle. This didn't turn out to be as neat as I hoped when I started, but I think it's still an improvement.
29fa927 to
1f9fa23
Compare
Contributor
Author
Collaborator
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 22, 2024
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#118714 ( Explanation that fields are being used when deriving `(Partial)Ord` on enums) - rust-lang#119710 (Improve `let_underscore_lock`) - rust-lang#119726 (Tweak Library Integer Division Docs) - rust-lang#119746 (rustdoc: hide modals when resizing the sidebar) - rust-lang#119986 (Fix error counting) - rust-lang#120194 (Shorten `#[must_use]` Diagnostic Message for `Option::is_none`) - rust-lang#120200 (Correct the anchor of an URL in an error message) - rust-lang#120203 (Replace `#!/bin/bash` with `#!/usr/bin/env bash` in rust-installer tests) - rust-lang#120212 (Give nnethercote more reviews) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 22, 2024
Rollup merge of rust-lang#119986 - nnethercote:fix-error-counting, r=compiler-errors,oli-obk Fix error counting There is some messiness in how errors get counted. Here are some cleanups. r? `@compiler-errors`
This file contains hidden or 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
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.
There is some messiness in how errors get counted. Here are some cleanups.
r? @compiler-errors