Skip to content

Fix error counting #119986

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jan 22, 2024
Merged

Fix error counting #119986

merged 3 commits into from
Jan 22, 2024

Conversation

nnethercote
Copy link
Contributor

There is some messiness in how errors get counted. Here are some cleanups.

r? @compiler-errors

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 15, 2024
@compiler-errors
Copy link
Member

Looks good to me on a first pass, but given that this touches track_errors and other error-counting logic, maybe this could use a second review by oli

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned compiler-errors Jan 18, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jan 18, 2024

Most track_errors are getting removed in #119869, the others in #119895

I guess inlining the last one is fine, I'm working on removing the error count check entirely anyway

@oli-obk
Copy link
Contributor

oli-obk commented Jan 19, 2024

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 ^^

@bors
Copy link
Collaborator

bors commented Jan 19, 2024

☔ 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.
@nnethercote
Copy link
Contributor Author

I rebased and removed the "Remove track_errors" commit because #119869 landed in the meantime and it overlaps a great deal. (Which means the one use of track_errors is still present.)

@bors r=compiler-errors,oli-obk

@bors
Copy link
Collaborator

bors commented Jan 21, 2024

📌 Commit 1f9fa23 has been approved by compiler-errors,oli-obk

It is now in the queue for this repository.

@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 Jan 21, 2024
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
@bors bors merged commit bef2e85 into rust-lang:master Jan 22, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 22, 2024
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`
@nnethercote nnethercote deleted the fix-error-counting branch January 22, 2024 20:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants