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.

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

replace track_errors usages with bubbling up ErrorGuaranteed #119869

Merged
merged 5 commits into from
Jan 19, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 11, 2024

more of the same as #117449 (removing track_errors)

@rustbot
Copy link
Collaborator

rustbot commented Jan 11, 2024

r? @BoxyUwU

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

@rustbot rustbot added 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. labels Jan 11, 2024
Comment on lines -83 to +91
return;
// This is super fishy, but our current `rustc_hir_analysis::check_crate` pipeline depends on
// `type_of` having been called much earlier, and thus this value being read from cache.
// Compilation must continue in order for other important diagnostics to keep showing up.
return Ok(());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new errors in the tests happen because of this hack.

  • If we track_errors the type_of call above, we'd get zero diagnostics changes.
  • If we use error_reported()? on the type returned from type_of, we lose lots of useful diagnostics
  • If we ignore the Result returned from check_mod_impl_wf, we get lots of new diagnostics, sometimes not very useful.

We can change this easily in the future, but I'd rather write separate PRs to compare, and not do it in this PR.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 11, 2024

r? @matthewjasper

Comment on lines +182 to +180
// FIXME(matthewjasper) We shouldn't need to use `track_errors` anywhere in this function
// or the compiler in general.
res.and(tcx.sess.track_errors(|| {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2024

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2024

📌 Commit 18e6643 has been approved by matthewjasper

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 18, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119582 (bootstrap: handle vendored sources when remapping crate paths)
 - rust-lang#119730 (docs: fix typos)
 - rust-lang#119828 (Improved collapse_debuginfo attribute, added command-line flag)
 - rust-lang#119869 (replace `track_errors` usages with bubbling up `ErrorGuaranteed`)
 - rust-lang#120037 (Remove `next_root_ty_var`)
 - rust-lang#120094 (tests/ui/asm/inline-syntax: adapt for LLVM 18)
 - rust-lang#120096 (Set RUSTC_BOOTSTRAP=1 consistently)
 - rust-lang#120101 (change `.unwrap()` to `?` on write where `fmt::Result` is returned)
 - rust-lang#120102 (Fix typo in munmap_partial.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@oli-obk oli-obk mentioned this pull request Jan 18, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119582 (bootstrap: handle vendored sources when remapping crate paths)
 - rust-lang#119730 (docs: fix typos)
 - rust-lang#119828 (Improved collapse_debuginfo attribute, added command-line flag)
 - rust-lang#119869 (replace `track_errors` usages with bubbling up `ErrorGuaranteed`)
 - rust-lang#120037 (Remove `next_root_ty_var`)
 - rust-lang#120094 (tests/ui/asm/inline-syntax: adapt for LLVM 18)
 - rust-lang#120096 (Set RUSTC_BOOTSTRAP=1 consistently)
 - rust-lang#120101 (change `.unwrap()` to `?` on write where `fmt::Result` is returned)
 - rust-lang#120102 (Fix typo in munmap_partial.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fa52eda into rust-lang:master Jan 19, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2024
Rollup merge of rust-lang#119869 - oli-obk:track_errors2, r=matthewjasper

replace `track_errors` usages with bubbling up `ErrorGuaranteed`

more of the same as rust-lang#117449 (removing `track_errors`)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2024
…asper

Remove `track_errors` entirely

follow up to rust-lang#119869

r? `@matthewjasper`

There are some diagnostic changes adding new diagnostics or not emitting some anymore. We can improve upon that in follow-up work imo.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2024
Rollup merge of rust-lang#119895 - oli-obk:track_errors_3, r=matthewjasper

Remove `track_errors` entirely

follow up to rust-lang#119869

r? `@matthewjasper`

There are some diagnostic changes adding new diagnostics or not emitting some anymore. We can improve upon that in follow-up work imo.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 25, 2024
…sper

replace `track_errors` usages with bubbling up `ErrorGuaranteed`

more of the same as rust-lang#117449 (removing `track_errors`)
feliperodri added a commit to model-checking/kani that referenced this pull request Feb 8, 2024
Related PRs so far:

- rust-lang/rust#119869
- rust-lang/rust#120080
- rust-lang/rust#120128
- rust-lang/rust#119369
- rust-lang/rust#116672

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.

---------

Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: tautschnig <tautschnig@users.noreply.github.com>
Co-authored-by: Qinheping Hu <qinhh@amazon.com>
Co-authored-by: Michael Tautschnig <tautschn@amazon.com>
Co-authored-by: Felipe R. Monteiro <felisous@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants