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.

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

Remove track_errors entirely #119895

Merged
merged 2 commits into from
Jan 25, 2024
Merged

Remove track_errors entirely #119895

merged 2 commits into from
Jan 25, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 12, 2024 •
edited
Loading

follow up to #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.

@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 12, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
This was referenced Jan 17, 2024
Merged
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 20, 2024
@GuillaumeGomez
Rollup merge of rust-lang#120059 - oli-obk:const_arg_type_mismatch, r…
c4ad0e7 
…=compiler-errors

Make generic const type mismatches not hide trait impls from the trait solver

pulled out of rust-lang#119895

It does improve diagnostics somewhat, but also causes some extraneous diagnostics in potentially misleading order.

The issue was that a const type mismatch, instead of reporting an error, would silently poison the constant, only for that information to be thrown away and the impl to be treated as "not matching". In rust-lang#119895 this would cause ICEs as well as errors on impls stating that the impl needs to exist for itself to be valid.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 22, 2024
@matthiaskrgr
Rollup merge of rust-lang#120059 - oli-obk:const_arg_type_mismatch, r…
071938a 
…=compiler-errors

Make generic const type mismatches not hide trait impls from the trait solver

pulled out of rust-lang#119895

It does improve diagnostics somewhat, but also causes some extraneous diagnostics in potentially misleading order.

The issue was that a const type mismatch, instead of reporting an error, would silently poison the constant, only for that information to be thrown away and the impl to be treated as "not matching". In rust-lang#119895 this would cause ICEs as well as errors on impls stating that the impl needs to exist for itself to be valid.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 22, 2024
@matthiaskrgr
Rollup merge of rust-lang#120059 - oli-obk:const_arg_type_mismatch, r…
d942357 
…=compiler-errors

Make generic const type mismatches not hide trait impls from the trait solver

pulled out of rust-lang#119895

It does improve diagnostics somewhat, but also causes some extraneous diagnostics in potentially misleading order.

The issue was that a const type mismatch, instead of reporting an error, would silently poison the constant, only for that information to be thrown away and the impl to be treated as "not matching". In rust-lang#119895 this would cause ICEs as well as errors on impls stating that the impl needs to exist for itself to be valid.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
@rust-timer
Unrolled build for rust-lang#120059
a752472 
Rollup merge of rust-lang#120059 - oli-obk:const_arg_type_mismatch, r=compiler-errors

Make generic const type mismatches not hide trait impls from the trait solver

pulled out of rust-lang#119895

It does improve diagnostics somewhat, but also causes some extraneous diagnostics in potentially misleading order.

The issue was that a const type mismatch, instead of reporting an error, would silently poison the constant, only for that information to be thrown away and the impl to be treated as "not matching". In rust-lang#119895 this would cause ICEs as well as errors on impls stating that the impl needs to exist for itself to be valid.
@bors
Copy link
Contributor

bors commented Jan 22, 2024

☔ The latest upstream changes (presumably #120242) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk marked this pull request as ready for review January 23, 2024 15:24
matthewjasper
matthewjasper reviewed Jan 24, 2024
View reviewed changes
compiler/rustc_middle/src/query/mod.rs Show resolved Hide resolved
tests/ui/associated-types/issue-38821.stderr
@@ -17,6 +17,272 @@ help: consider further restricting the associated type
LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
| +++++++++++++++++++++++++++++++++++++++

error: aborting due to 1 previous error
error[E0277]: the trait bound `<Col as Expression>::SqlType: NotNull` is not satisfied
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know what this is caused by?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just lots of occurrences within the bodies of the Debug and Clone impls. Before this PR we didn't continue to the bodies due to the previous errors. All the errors are deduplicated, so this isn't user-visibble

@rustbot rustbot added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Jan 24, 2024
@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 24, 2024

📌 Commit cc34dc2 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 24, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 25, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2024
@bors
Auto merge of rust-lang#120335 - matthiaskrgr:rollup-2a0y3rd, r=matth…
5bd5d21 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#119305 (Add `AsyncFn` family of traits)
 - rust-lang#119389 (Provide more context on recursive `impl` evaluation overflow)
 - rust-lang#119895 (Remove `track_errors` entirely)
 - rust-lang#120230 (Assert that a single scope is passed to `for_scope`)
 - rust-lang#120278 (Remove --fatal-warnings on wasm targets)
 - rust-lang#120292 (coverage: Dismantle `Instrumentor` and flatten span refinement)
 - rust-lang#120315 (On E0308 involving `dyn Trait`, mention trait objects)
 - rust-lang#120317 (pattern_analysis: Let `ctor_sub_tys` return any Iterator they want)
 - rust-lang#120318 (pattern_analysis: Reuse most of the `DeconstructedPat` `Debug` impl)
 - rust-lang#120325 (rustc_data_structures: use either instead of itertools)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0c45e3c into rust-lang:master Jan 25, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2024
@rust-timer
Unrolled build for rust-lang#119895
b59ed7b 
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.
@oli-obk oli-obk deleted the track_errors_3 branch January 25, 2024 11:36
@oli-obk oli-obk mentioned this pull request Jan 25, 2024
Merged
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 25, 2024
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 30, 2024
@GuillaumeGomez
Rollup merge of rust-lang#120342 - oli-obk:track_errors6, r=nnethercote
b28e6f1 
Remove various `has_errors` or `err_count` uses

follow up to rust-lang#119895

r? `@nnethercote` since you recently did something similar.

There are so many more of these, but I wanted to get a PR out instead of growing the commit list indefinitely. The commits all work on their own and can be reviewed commit by commit.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2024
@rust-timer
Unrolled build for rust-lang#120342
a422946 
Rollup merge of rust-lang#120342 - oli-obk:track_errors6, r=nnethercote

Remove various `has_errors` or `err_count` uses

follow up to rust-lang#119895

r? `@nnethercote` since you recently did something similar.

There are so many more of these, but I wanted to get a PR out instead of growing the commit list indefinitely. The commits all work on their own and can be reviewed commit by commit.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Feb 8, 2024
@GuillaumeGomez
Rollup merge of #120342 - oli-obk:track_errors6, r=nnethercote
f0dbf68 
Remove various `has_errors` or `err_count` uses

follow up to rust-lang/rust#119895

r? `@nnethercote` since you recently did something similar.

There are so many more of these, but I wanted to get a PR out instead of growing the commit list indefinitely. The commits all work on their own and can be reviewed commit by commit.
@compiler-errors compiler-errors mentioned this pull request Sep 23, 2024
Merged
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 24, 2024
@tgross35
Rollup merge of rust-lang#130764 - compiler-errors:inherent, r=estebank
3b45f8f 
Separate collection of crate-local inherent impls from error tracking

rust-lang#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until rust-lang#121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.

Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in rust-lang#127798.

This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.

This fixes rust-lang#127798.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 25, 2024
@rust-timer
Unrolled build for rust-lang#130764
58192ec 
Rollup merge of rust-lang#130764 - compiler-errors:inherent, r=estebank

Separate collection of crate-local inherent impls from error tracking

rust-lang#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until rust-lang#121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.

Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in rust-lang#127798.

This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.

This fixes rust-lang#127798.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 3, 2024
@tgross35
Rollup merge of rust-lang#130764 - compiler-errors:inherent, r=estebank
e691743 
Separate collection of crate-local inherent impls from error tracking

rust-lang#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until rust-lang#121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.

Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in rust-lang#127798.

This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.

This fixes rust-lang#127798.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewjasper matthewjasper matthewjasper left review comments

@nnethercote nnethercote nnethercote left review comments

Assignees

@matthewjasper matthewjasper

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.
Projects
None yet
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @rust-log-analyzer @bors @matthewjasper @nnethercote @rustbot