Avoid ICES after reporting errors on erroneous patterns by oli-obk · Pull Request #126320 · rust-lang/rust

oli-obk · 2024-06-12T11:05:07Z

fixes #109812
fixes #125914
fixes #124004

rustbot · 2024-06-12T11:05:15Z

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

oli-obk · 2024-06-12T11:05:45Z

pulled out of #126316

lcnr

👍 sgtm, though I would love to see some slightly more involved cleanup here while you're already at it 😊

compiler/rustc_hir_typeck/src/pat.rs

lcnr · 2024-06-12T12:07:00Z

r? @lcnr

lcnr

very nice ❤️

r=me after nits

lcnr · 2024-06-12T19:12:22Z

compiler/rustc_hir_typeck/src/demand.rs

+            Ok(ty) => ty,
+            Err(err) => {
+                err.emit();
+                expected


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.

I did have that, the result is not great. I should document it here

lcnr · 2024-06-12T19:13:46Z

compiler/rustc_hir_typeck/src/demand.rs


        let e = match self.coerce(expr, checked_ty, expected, allow_two_phase, None) {
-            Ok(ty) => return (ty, None),
+            Ok(ty) => return Ok(ty),


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

Oh, we didn't emit the error initially, so I didn't change it, good catch

Ah nevermind, we're not actually emitting the error here, we're just creating the diagnostic

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.

lcnr · 2024-06-12T19:15:29Z

tests/ui/pattern/missing_lifetime.rs

+//! Instead of actually analyzing the erroneous patterns,
+//! we instead stop after typeck where errors are already
+//! reported.
+//!


Suggested change

//!

oli-obk · 2024-06-13T06:16:26Z

@bors r=lcnr

bors · 2024-06-13T06:16:29Z

📌 Commit 7566307 has been approved by lcnr

It is now in the queue for this repository.

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

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

Avoid ICEs after bad patterns, for the other syntactic variants This PR introduces changes equivalent to the ones in rust-lang#126320, but also for struct and tuple patterns, instead of tuple struct patterns only. Fixes rust-lang#150507.

Rollup merge of #150979 - typeck-pat, r=lcnr Avoid ICEs after bad patterns, for the other syntactic variants This PR introduces changes equivalent to the ones in #126320, but also for struct and tuple patterns, instead of tuple struct patterns only. Fixes #150507.

rustbot assigned cjgillot Jun 12, 2024

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 Jun 12, 2024

lcnr reviewed Jun 12, 2024

View reviewed changes

compiler/rustc_hir_typeck/src/pat.rs Outdated Show resolved Hide resolved

rustbot assigned lcnr and unassigned cjgillot Jun 12, 2024

This comment has been minimized.

Sign in to view

oli-obk force-pushed the pat_ice branch from c84615c to b446e74 Compare June 12, 2024 16:00

lcnr reviewed Jun 12, 2024

View reviewed changes

oli-obk added 5 commits June 13, 2024 06:14

Avoid follow-up errors on erroneous patterns

2733b8a

Replace some Option<Diag> with Result<(), Diag>

a621701

Replace some Option<Diag> with Result<(), Diag>

ece3e3e

Replace some Option<Diag> with Result<(), Diag>

e8d6170

Replace a bool with a Result<(), ErrorGuaranteed>

7566307

oli-obk force-pushed the pat_ice branch from b446e74 to 7566307 Compare June 13, 2024 06:16

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 Jun 13, 2024

fmease added a commit to fmease/rust that referenced this pull request Jun 13, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

2a7a689

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

fmease mentioned this pull request Jun 13, 2024

Rollup of 5 pull requests #126397

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

29cb7a9

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

matthiaskrgr mentioned this pull request Jun 13, 2024

Rollup of 7 pull requests #126412

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

758f890

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

matthiaskrgr mentioned this pull request Jun 13, 2024

Rollup of 8 pull requests #126433

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

5bb74de

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

matthiaskrgr mentioned this pull request Jun 13, 2024

Rollup of 9 pull requests #126438

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

dd3a02f

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

matthiaskrgr mentioned this pull request Jun 13, 2024

Rollup of 8 pull requests #126441

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024

Rollup merge of rust-lang#126320 - oli-obk:pat_ice, r=lcnr

5725958

Avoid ICES after reporting errors on erroneous patterns fixes rust-lang#109812 fixes rust-lang#125914 fixes rust-lang#124004

matthiaskrgr mentioned this pull request Jun 14, 2024

Rollup of 10 pull requests #126458

Closed

matthiaskrgr mentioned this pull request Jun 14, 2024

Rollup of 5 pull requests #126462

Merged

bors merged commit 2b3fb62 into rust-lang:master Jun 14, 2024

rustbot added this to the 1.81.0 milestone Jun 14, 2024

matthiaskrgr mentioned this pull request Jun 20, 2024

ICE: typeck: no index for a field #126744

Closed

meithecatte mentioned this pull request Jan 11, 2026

Avoid ICEs after bad patterns, for the other syntactic variants #150979

Merged

Uh oh!

Conversation

oli-obk commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 12, 2024

Uh oh!

oli-obk commented Jun 12, 2024

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lcnr commented Jun 12, 2024

Uh oh!

This comment has been minimized.

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Jun 13, 2024

Uh oh!

bors commented Jun 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

oli-obk commented Jun 12, 2024 •

edited

Loading