-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Avoid silencing relevant follow-up errors #117449
Conversation
This comment has been minimized.
This comment has been minimized.
edc98bd
to
333aab4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see someone working on this.
There are a couple of cases where I think the new additional error messages are not helpful. It would be good to either have them fixed in this PR or have a clear proposal to fix them.
|
||
#![feature(type_alias_impl_trait)] | ||
|
||
type Pointer<T> = impl std::ops::Deref<Target = T>; | ||
|
||
fn test() -> Pointer<_> { | ||
//~^ ERROR: the placeholder `_` is not allowed within types | ||
//~| ERROR: non-defining opaque type use in defining scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder here should suppress this error or inference errors.
@@ -2,6 +2,7 @@ | |||
|
|||
#[rustc_outlives] | |||
struct Foo<'a, 'b, T> { //~ ERROR rustc_outlives | |||
//~^ ERROR rustc_outlives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this error is emitted in two places doing fairly similar things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a compiler debugging tool that I think doesn't matter if the UX is weird
@@ -5,7 +5,7 @@ | |||
async fn copy() -> Result<()> | |||
//~^ ERROR enum takes 2 generic arguments | |||
{ | |||
Ok(()) | |||
Ok(()) //~ ERROR: type annotations needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Various incorrect number of generic arguments errors should suppress later inference and constrained variable errors.
Yea I'd like to fix those follow up errors, but I can't without this PR or without doing fragile const eval crimes. Due to the amount of errors that change here, I don't think we should do more in this PR, but instead land it and follow up on the not helpful errors in separate PRs. I'll open a larger issue around this topic, as it's something we're already working on, but not in a structured way. |
Fair enough. r=me with the comment in |
@bors r=matthewjasper |
…=matthewjasper Avoid silencing relevant follow-up errors r? `@matthewjasper` This PR only adds new errors to tests that are already failing and fixes one ICE. Several tests were changed to not emit new errors. I believe all of them were faulty tests, and not explicitly testing for the code that had new errors.
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
… r=matthewjasper Avoid silencing relevant follow-up errors r? `@matthewjasper` This PR only adds new errors to tests that are already failing and fixes one ICE. Several tests were changed to not emit new errors. I believe all of them were faulty tests, and not explicitly testing for the code that had new errors.
☔ The latest upstream changes (presumably #117817) made this pull request unmergeable. Please resolve the merge conflicts. |
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#118962 (Annotate some bugs) - rust-lang#118969 (coverage: Use `Waker::noop` in async tests) - rust-lang#118974 (Annotate panic! reasons during enum layout) Failed merges: - rust-lang#111658 (Refactor pre-getopts command line argument handling) - rust-lang#117449 (Avoid silencing relevant follow-up errors) r? `@ghost` `@rustbot` modify labels: rollup
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
At a quick glance, it seems this is waiting for a rebase and then it's ready for merge, am I right? Switching to waiting on author. @rustbot author |
cd56d91
to
594b5aa
Compare
@bors r=matthewjasper |
Finished benchmarking commit (9480767): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 666.209s -> 667.55s (0.20%) |
Hmm, looks like this causes ICEs that were previously hidden by errors. :/ |
imo that's a good thing. We were always playing with fire by using I'll happily do more ICE squashing. |
…, r=compiler-errors Silence some follow-up errors [1/x] this is one piece of the requested cleanups from rust-lang#117449 When we use `-> impl SomeTrait<_>` as a return type, we are both using the "infer return type suggestion" code path, and the infer opaque type code path within the same function. That can lead to confusing diagnostics, so silence all opaque type diagnostics in that case.
Rollup merge of rust-lang#119803 - oli-obk:even_more_follow_up_errors, r=compiler-errors Silence some follow-up errors [1/x] this is one piece of the requested cleanups from rust-lang#117449 When we use `-> impl SomeTrait<_>` as a return type, we are both using the "infer return type suggestion" code path, and the infer opaque type code path within the same function. That can lead to confusing diagnostics, so silence all opaque type diagnostics in that case.
…ler-errors Silence some follow-up errors [1/x] this is one piece of the requested cleanups from rust-lang/rust#117449 When we use `-> impl SomeTrait<_>` as a return type, we are both using the "infer return type suggestion" code path, and the infer opaque type code path within the same function. That can lead to confusing diagnostics, so silence all opaque type diagnostics in that case.
…2, r=estebank Silence some follow-up errors [2/x] this is one piece of the requested cleanups from rust-lang#117449 the `type_of` query frequently uses astconv to convert a `hir::Ty` to a `ty::Ty`. This process is infallible, but may produce errors as it goes. All the error reporting sites that had access to the `ItemCtxt` are now tainting it, causing `type_of` to return a `ty::Error` instead of anything else.
…2, r=estebank Silence some follow-up errors [2/x] this is one piece of the requested cleanups from rust-lang#117449 the `type_of` query frequently uses astconv to convert a `hir::Ty` to a `ty::Ty`. This process is infallible, but may produce errors as it goes. All the error reporting sites that had access to the `ItemCtxt` are now tainting it, causing `type_of` to return a `ty::Error` instead of anything else.
Rollup merge of rust-lang#119813 - oli-obk:even_more_follow_up_errors2, r=estebank Silence some follow-up errors [2/x] this is one piece of the requested cleanups from rust-lang#117449 the `type_of` query frequently uses astconv to convert a `hir::Ty` to a `ty::Ty`. This process is infallible, but may produce errors as it goes. All the error reporting sites that had access to the `ItemCtxt` are now tainting it, causing `type_of` to return a `ty::Error` instead of anything else.
…3, r=compiler-errors Silence some follow-up errors [3/x] this is one piece of the requested cleanups from rust-lang#117449 Keep error types around, even in obligations. These help silence follow-up errors, as we now figure out that some types (most notably inference variables) are equal to an error type. But it also allows figuring out more types in the presence of errors, possibly causing more errors.
…3, r=compiler-errors Silence some follow-up errors [3/x] this is one piece of the requested cleanups from rust-lang#117449 Keep error types around, even in obligations. These help silence follow-up errors, as we now figure out that some types (most notably inference variables) are equal to an error type. But it also allows figuring out more types in the presence of errors, possibly causing more errors.
Rollup merge of rust-lang#119818 - oli-obk:even_more_follow_up_errors3, r=compiler-errors Silence some follow-up errors [3/x] this is one piece of the requested cleanups from rust-lang#117449 Keep error types around, even in obligations. These help silence follow-up errors, as we now figure out that some types (most notably inference variables) are equal to an error type. But it also allows figuring out more types in the presence of errors, possibly causing more errors.
…sper replace `track_errors` usages with bubbling up `ErrorGuaranteed` more of the same as rust-lang#117449 (removing `track_errors`)
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`)
replace `track_errors` usages with bubbling up `ErrorGuaranteed` more of the same as rust-lang/rust#117449 (removing `track_errors`)
replace `track_errors` usages with bubbling up `ErrorGuaranteed` more of the same as rust-lang/rust#117449 (removing `track_errors`)
r? @matthewjasper
This PR only adds new errors to tests that are already failing and fixes one ICE.
Several tests were changed to not emit new errors. I believe all of them were faulty tests, and not explicitly testing for the code that had new errors.