feat: `min_exhaustive_patterns` #17853

ShoyuVanilla · 2024-08-11T13:50:30Z

Resolves #17851

ShoyuVanilla · 2024-08-11T13:51:23Z

crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs

@@ -328,7 +325,7 @@ impl<'db> PatCx for MatchCheckCtx<'db> {
        self.exhaustive_patterns
    }
    fn is_min_exhaustive_patterns_feature_on(&self) -> bool {
-        self.min_exhaustive_patterns
+        true


Should I bump up ra-ap-rustc_pattern_analysis to latest instead?

There is a PR upstream enabling the in-tree pattern analysis in the rust repo (to prevent us from going out of sync), so these two PRs will likely collide. Might be best if either PR waits on the other (and if we pick this one here first then yes we should bump the dependency here), cc @Nadrieril

Since the rust-lang/rust#128922 is in homu queue, maybe I should wait for it?

ShoyuVanilla · 2024-08-11T13:53:59Z

crates/hir-ty/src/diagnostics/expr.rs

+            _ => true,
+        }
+    }
+


This dance is the compressed version of
https://github.com/rust-lang/rust/blob/c9bd03cb724e13cca96ad320733046cbdb16fbbe/compiler/rustc_mir_build/src/thir/cx/expr.rs
and then
https://github.com/rust-lang/rust/blob/c9bd03cb724e13cca96ad320733046cbdb16fbbe/compiler/rustc_mir_build/src/thir/pattern/check_match.rs#L288

ShoyuVanilla · 2024-08-11T13:55:47Z

crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs

@@ -95,8 +93,7 @@ impl<'db> MatchCheckCtx<'db> {
            }
        }

-        // FIXME: Determine place validity correctly. For now, err on the safe side.
-        let place_validity = PlaceValidity::MaybeInvalid;
+        let place_validity = PlaceValidity::from_bool(known_valid_scrutinee.unwrap_or(true));


This unwrap_or thing is from rustc;
https://github.com/rust-lang/rust/blob/c9bd03cb724e13cca96ad320733046cbdb16fbbe/compiler/rustc_mir_build/src/thir/pattern/check_match.rs#L372-L373

ShoyuVanilla · 2024-08-11T14:02:30Z

crates/ide-diagnostics/src/handlers/non_exhaustive_let.rs

+    let Ok(_y) = x;
+}
+"#,
+        );


The reason that this test doesn't have a pointer deref case is because the following code;

fn test(ptr: *const Result<i32, !>) { unsafe { let Ok(_x) = *ptr; } }

is getting a block with no stmts but tail one in here(thus, no diagnostic error),

rust-analyzer/crates/hir-ty/src/diagnostics/expr.rs

Lines 256 to 257 in 0daeb5c

fn validate_block(&mut self, db: &dyn HirDatabase, expr: &Expr) {

let Expr::Block { statements, .. } = expr else { return };

while the following is getting a block with a single stmt without tail 🤔

fn test(x: Result<i32, &'static !>) { let Ok(_y) = x; }

I'll make a more deep inspection and file this as a new issue

is getting a block with no stmts but tail one in here(thus, no diagnostic error),

oh no that sounds very wrong

Yes, it looks very wrong 🤔 I'll check the entire lowering process from AST to hir once I get back home

This is because of #17865

I got into this 😂 rust-lang/rust#129009

Veykril · 2024-08-11T15:19:52Z

No that pr got removed from the queue as it doesn't build, so I guess if you wanna bump our crates io version first we can swap to in tree in the rust repo as a follow up after we've synced this back

…

On Sun, Aug 11, 2024, 4:55 PM Shoyu Vanilla ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In crates/hir-ty/src/diagnostics/match_check/pat_analysis.rs <#17853 (comment)> : > @@ -328,7 +325,7 @@ impl<'db> PatCx for MatchCheckCtx<'db> { self.exhaustive_patterns } fn is_min_exhaustive_patterns_feature_on(&self) -> bool { - self.min_exhaustive_patterns + true Since the rust-lang/rust#128922 <rust-lang/rust#128922> is in homu queue, maybe I should wait for it? — Reply to this email directly, view it on GitHub <#17853 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4VNS23RS47SJL3BM3XIE3ZQ53NLAVCNFSM6AAAAABMKY2JMKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMZRHAZTEOBVGU> . You are receiving this because you commented.Message ID: ***@***.***>

ShoyuVanilla · 2024-08-11T15:27:29Z

In either case, I'll continue on this about 20 hours later when I get back from work because it's midnight here 😅

Veykril · 2024-08-11T17:03:22Z

No rush

ShoyuVanilla · 2024-08-12T14:29:14Z

Well, the latest ra-ap-rustc_pattern_analysis, 0.62.0 does not pointing at 853255e, which is quite strange that the roll-up merge had been completed about two hours before scheduled rustc-auto-publish's publish action 🤔
Actually, the publish workflow wasn't triggered last sunday

Veykril · 2024-08-12T14:34:07Z

Eh, the autopublish action got frozen by github again due to repo inactivity

fix: Missing non-exhaustive let diagnostics inside async or unsafe block The reason that this test doesn't have a pointer deref case is because the following code; ```rust fn test(ptr: *const Result<i32, !>) { unsafe { let Ok(_x) = *ptr; } } ``` is getting a block with no stmts but tail one in here(thus, no diagnostic error), https://github.com/rust-lang/rust-analyzer/blob/0daeb5c0b05cfdf2101b0f078c27539099bf38e6/crates/hir-ty/src/diagnostics/expr.rs#L256-L257 while the following is getting a block with a single stmt without tail 🤔 ```rust fn test(x: Result<i32, &'static !>) { let Ok(_y) = x; } ``` I'll make a more deep inspection and file this as a new issue _Originally posted by `@ShoyuVanilla` in #17853 (comment)

Veykril · 2024-08-12T14:34:59Z

Kicked off a new one https://github.com/rust-analyzer/rustc-auto-publish/actions/workflows/main.yml

bors · 2024-08-12T14:49:01Z

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

Veykril · 2024-08-13T06:23:30Z

Cargo.toml

 ra-ap-rustc_parse_format = { version = "0.53.0", default-features = false }
 ra-ap-rustc_index = { version = "0.53.0", default-features = false }
 ra-ap-rustc_abi = { version = "0.53.0", default-features = false }


Do bump the other three as well otherwise we get some duplicate transitive dependencies

Veykril · 2024-08-13T06:25:30Z

@bors delegate+

bors · 2024-08-13T06:25:33Z

✌️ @ShoyuVanilla, you can now approve this pull request!

If @Veykril told you to "r=me" after making some further change, please make that change, then do @bors r=@Veykril

ShoyuVanilla · 2024-08-13T14:17:07Z

@bors r+

bors · 2024-08-13T14:17:09Z

📌 Commit 588fa2c has been approved by ShoyuVanilla

It is now in the queue for this repository.

bors · 2024-08-13T14:18:18Z

⌛ Testing commit 588fa2c with merge e25abba...

bors · 2024-08-13T14:32:04Z

☀️ Test successful - checks-actions
Approved by: ShoyuVanilla
Pushing e25abba to master...

fix: Missing non-exhaustive let diagnostics inside async or unsafe block The reason that this test doesn't have a pointer deref case is because the following code; ```rust fn test(ptr: *const Result<i32, !>) { unsafe { let Ok(_x) = *ptr; } } ``` is getting a block with no stmts but tail one in here(thus, no diagnostic error), https://github.com/rust-lang/rust-analyzer/blob/0daeb5c0b05cfdf2101b0f078c27539099bf38e6/crates/hir-ty/src/diagnostics/expr.rs#L256-L257 while the following is getting a block with a single stmt without tail 🤔 ```rust fn test(x: Result<i32, &'static !>) { let Ok(_y) = x; } ``` I'll make a more deep inspection and file this as a new issue _Originally posted by `@ShoyuVanilla` in rust-lang/rust-analyzer#17853 (comment)

Nadrieril · 2024-08-13T21:06:59Z

Just had time to take a look, looks good to me! Thx for taking the time to do this

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2024

ShoyuVanilla commented Aug 11, 2024

View reviewed changes

ShoyuVanilla mentioned this pull request Aug 12, 2024

fix: Missing non-exhaustive let diagnostics inside async or unsafe block #17865

Merged

ShoyuVanilla force-pushed the min-exhaustive-pat branch from a2154da to 1f25210 Compare August 12, 2024 15:24

Veykril approved these changes Aug 13, 2024

View reviewed changes

Veykril mentioned this pull request Aug 13, 2024

rust-analyzer: use in-tree pattern_analysis crate rust-lang/rust#128922

Merged

ShoyuVanilla added 3 commits August 13, 2024 23:10

feat: `min-exhaustive-patterns

5316ba9

Temporarily remove non-working test case

a7bc556

Bump rustc_pattern_analysis

588fa2c

ShoyuVanilla force-pushed the min-exhaustive-pat branch from 40ef939 to 588fa2c Compare August 13, 2024 14:15

bors merged commit e25abba into rust-lang:master Aug 13, 2024
11 checks passed

ShoyuVanilla deleted the min-exhaustive-pat branch August 13, 2024 14:38

lnicola changed the title ~~feat: min-exhaustive-patterns~~ feat: min_exhaustive_patterns Aug 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `min_exhaustive_patterns` #17853

feat: `min_exhaustive_patterns` #17853

ShoyuVanilla commented Aug 11, 2024

ShoyuVanilla Aug 11, 2024

Veykril Aug 11, 2024

ShoyuVanilla Aug 11, 2024 •

edited

Loading

ShoyuVanilla Aug 11, 2024 •

edited

Loading

ShoyuVanilla Aug 11, 2024

ShoyuVanilla Aug 11, 2024 •

edited

Loading

Veykril Aug 12, 2024

ShoyuVanilla Aug 12, 2024

ShoyuVanilla Aug 12, 2024 •

edited

Loading

ShoyuVanilla Aug 12, 2024

Veykril commented Aug 11, 2024 via email

ShoyuVanilla commented Aug 11, 2024

Veykril commented Aug 11, 2024

ShoyuVanilla commented Aug 12, 2024

Veykril commented Aug 12, 2024

Veykril commented Aug 12, 2024

bors commented Aug 12, 2024

Veykril Aug 13, 2024

Veykril commented Aug 13, 2024

bors commented Aug 13, 2024

ShoyuVanilla commented Aug 13, 2024

bors commented Aug 13, 2024

bors commented Aug 13, 2024

bors commented Aug 13, 2024

Nadrieril commented Aug 13, 2024

	fn validate_block(&mut self, db: &dyn HirDatabase, expr: &Expr) {
	let Expr::Block { statements, .. } = expr else { return };

feat: min_exhaustive_patterns #17853

feat: min_exhaustive_patterns #17853

Conversation

ShoyuVanilla commented Aug 11, 2024

ShoyuVanilla Aug 11, 2024

Choose a reason for hiding this comment

Veykril Aug 11, 2024

Choose a reason for hiding this comment

ShoyuVanilla Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

ShoyuVanilla Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

ShoyuVanilla Aug 11, 2024

Choose a reason for hiding this comment

ShoyuVanilla Aug 11, 2024 • edited Loading

Choose a reason for hiding this comment

Veykril Aug 12, 2024

Choose a reason for hiding this comment

ShoyuVanilla Aug 12, 2024

Choose a reason for hiding this comment

ShoyuVanilla Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

ShoyuVanilla Aug 12, 2024

Choose a reason for hiding this comment

Veykril commented Aug 11, 2024 via email

ShoyuVanilla commented Aug 11, 2024

Veykril commented Aug 11, 2024

ShoyuVanilla commented Aug 12, 2024

Veykril commented Aug 12, 2024

Veykril commented Aug 12, 2024

bors commented Aug 12, 2024

Veykril Aug 13, 2024

Choose a reason for hiding this comment

Veykril commented Aug 13, 2024

bors commented Aug 13, 2024

ShoyuVanilla commented Aug 13, 2024

bors commented Aug 13, 2024

bors commented Aug 13, 2024

bors commented Aug 13, 2024

Nadrieril commented Aug 13, 2024

feat: `min_exhaustive_patterns` #17853

feat: `min_exhaustive_patterns` #17853

ShoyuVanilla Aug 11, 2024 •

edited

Loading

ShoyuVanilla Aug 11, 2024 •

edited

Loading

ShoyuVanilla Aug 11, 2024 •

edited

Loading

ShoyuVanilla Aug 12, 2024 •

edited

Loading