-
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 ICES after reporting errors on erroneous patterns #126320
Conversation
pulled out of #126316 |
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.
👍 sgtm, though I would love to see some slightly more involved cleanup here while you're already at it 😊
r? @lcnr |
This comment has been minimized.
This comment has been minimized.
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.
very nice ❤️
r=me after nits
Ok(ty) => ty, | ||
Err(err) => { | ||
err.emit(); | ||
expected |
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.
interesting that we return expected
instead of Ty::new_error
here 🤔 if someone wants to experiment with this, try changing it and run our UI test suite.
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.
I did have that, the result is not great. I should document it here
let expected = self.resolve_vars_with_obligations(expected); | ||
|
||
let e = match self.coerce(expr, checked_ty, expected, allow_two_phase, None) { | ||
Ok(ty) => return (ty, None), | ||
Ok(ty) => return Ok(ty), |
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.
we set_tainted_by_errors
in line 263 using a span_delayed_bug
and then emit the error further down in line 270. Feel like we should just flip the order and use the ErrorGuaranteed
from actually emitting something
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.
Oh, we didn't emit the error initially, so I didn't change it, good catch
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.
Ah nevermind, we're not actually emitting the error here, we're just creating the diagnostic
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.
Those functions don't have a great name, but that's a very common pattern throughout rustc. Maybe we should add an internal lint for methods with report
in their name that also return a Diag
.
tests/ui/pattern/missing_lifetime.rs
Outdated
//! Instead of actually analyzing the erroneous patterns, | ||
//! we instead stop after typeck where errors are already | ||
//! reported. | ||
//! |
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.
//! |
@bors r=lcnr |
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty) - rust-lang#122613 (Don't build a broken/untested profiler runtime on mingw targets) - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126399 (extend the check for LLVM build) Failed merges: - rust-lang#126388 (const-eval: make lint scope computation consistent) r? `@ghost` `@rustbot` modify labels: rollup
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty) - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.) - rust-lang#126315 (Add pub struct with allow(dead_code) into worklist) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126399 (extend the check for LLVM build) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty) - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.) - rust-lang#126315 (Add pub struct with allow(dead_code) into worklist) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126399 (extend the check for LLVM build) r? `@ghost` `@rustbot` modify labels: rollup
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty) - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.) - rust-lang#126315 (Add pub struct with allow(dead_code) into worklist) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126399 (extend the check for LLVM build) - rust-lang#126436 (Reduce rustdoc GUI tests flakyness) r? `@ghost` `@rustbot` modify labels: rollup
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty) - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.) - rust-lang#126315 (Add pub struct with allow(dead_code) into worklist) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126399 (extend the check for LLVM build) r? `@ghost` `@rustbot` modify labels: rollup
Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#124884 (place explicit lifetime bound after generic param) - rust-lang#126244 (Update fuchsia commit, and SDK to 21.20240610.2.1) - rust-lang#126270 (Migrate run make const fn mir) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126343 (Remove some msys2 utils) - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.) - rust-lang#126368 (Remove some unnecessary crate dependencies.) - rust-lang#126386 (Migrate `run-make/allow-non-lint-warnings-cmdline` to `rmake.rs`) - rust-lang#126449 (Fill out missing Windows support information) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates) - rust-lang#126244 (Update fuchsia commit, and SDK to 21.20240610.2.1) - rust-lang#126270 (Migrate run make const fn mir) - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns) - rust-lang#126449 (Fill out missing Windows support information) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004
fixes #109812
fixes #125914
fixes #124004