Further improve diagnostics for expressions in pattern position #123877

ShE3py · 2024-04-13T00:09:14Z

fn main() {
    match 'b' {
        y.0.0.1.z().f()? as u32 => {},
    }
}

Before:

error: expected one of `=>`, `@`, `if`, or `|`, found `.`
 --> src/main.rs:3:10
  |
3 |         y.0.0.1.z().f()? as u32 => {},
  |          ^ expected one of `=>`, `@`, `if`, or `|`

After:

error: expected a pattern, found an expression
 --> src/main.rs:3:9
  |
3 |         y.0.0.1.z().f()? as u32 => {},
  |         ^^^^^^^^^^^^^^^^^^^^^^^ arbitrary expressions are not allowed in patterns
  |
help: consider moving the expression to a match arm guard
  |
3 |         val if val == y.0.0.1.z().f()? as u32 => {},
  |         ~~~ +++++++++++++++++++++++++++++++++
help: consider extracting the expression into a `const`
  |
2 +     const VAL: /* Type */ = y.0.0.1.z().f()? as u32;
3 ~     match 'b' {
4 ~         VAL => {},
  |
help: consider wrapping the expression in an inline `const` (requires `#![feature(inline_const_pat)]`)
  |
3 |         const { y.0.0.1.z().f()? as u32 } => {},
  |         +++++++                         +

r? fmease
@rustbot label +A-diagnostics +A-parser +A-patterns +C-enhancement

compiler/rustc_errors/src/lib.rs

fmease · 2024-04-27T09:48:16Z

Currently vacationing for two more days, then I'm gonna review this PR.

ShE3py · 2024-04-29T08:09:37Z

Squashed the commits, I still have the let expr to do (e.g. let buf[0] = expr // remove this let) and some comments to update.

compiler/rustc_parse/src/parser/item.rs

bors · 2024-06-19T16:32:42Z

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

fmease

I've finished reviewing 3 out of 4 commits. Looking good so far. Gonna look at the 4th one in a sec. Sorry for taking so long ^^'

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/src/parser/pat.rs

fmease · 2024-07-11T20:17:53Z

compiler/rustc_parse/src/parser/pat.rs

+        else {
+            // We got a trailing method/operator, but that wasn't an expression.
+            return None;
+        };


Nice let-else!

fmease · 2024-07-11T20:34:13Z

tests/ui/parser/recover/recover-pat-exprs.stderr

+error: expected a pattern, found an expression
+  --> $DIR/recover-pat-exprs.rs:58:9
+   |
+LL |         x.sqrt() @ .. => (),
+   |         ^^^^^^^^ arbitrary expressions are not allowed in patterns


Btw, if you'd like to suppress this one, we could pass a param RecoverExprInPat::{Yes, No} to parse_pat…

fmease · 2024-07-11T20:39:45Z

tests/ui/parser/recover/recover-pat-exprs.stderr

+error: expected a pattern, found an expression
+  --> $DIR/recover-pat-exprs.rs:43:10
+   |
+LL |         (1 + 2) * 3 => (),
+   |          ^^^^^ arbitrary expressions are not allowed in patterns
+
+error: expected a pattern, found an expression
+  --> $DIR/recover-pat-exprs.rs:43:9
+   |
+LL |         (1 + 2) * 3 => (),
+   |         ^^^^^^^^^^^ arbitrary expressions are not allowed in patterns


Note to self: Ah, right. If we were to reparse the tokens as an expr when failing to parse a pat (starting right before the first (), then we would only emit a single error for (1 + 2) * 3 in pat position but we don't follow this approach since that would require creating a snapshot upfront potentially in the happy path very likely incurring a perf cost (it's been a while, trying to remember all the context).

fmease · 2024-07-11T20:41:52Z

tests/ui/parser/recover/recover-pat-exprs.rs

Very nicely structured test file 👏

compiler/rustc_parse/src/parser/pat.rs

bors · 2024-07-15T22:24:29Z

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

compiler/rustc_parse/messages.ftl

compiler/rustc_parse/src/errors.rs

compiler/rustc_parse/messages.ftl

tests/ui/parser/bad-name.stderr

fmease · 2024-08-20T01:56:10Z

Btw, I will gladly insta-approve a PR that just contains the first two commits (a9fbb22 and f8b3046) which are independent of expr-in-pats :)

ShE3py · 2024-09-05T16:47:12Z

Btw, I will gladly insta-approve a PR that just contains the first two commits (a9fbb22 and f8b3046) which are independent of expr-in-pats :)

I was on vacation and I wanted to move some comments (i.e. 2133219), I can split now if that's desirable.

I’ve reverted the “error reducing” for parse_pat_tuple_or_parens so as not to edit the stashing methods;

fn main() {
    let Some((1 + 2) * 3) = None::<i32>;
}

error: expected a pattern, found an expression
 --> src/main.rs:2:14
  |
2 |     let Some((1 + 2) * 3) = None::<i32>;
  |              ^^^^^^^^^^^ arbitrary expressions are not allowed in patterns

error: expected a pattern, found an expression
 --> src/main.rs:2:15
  |
2 |     let Some((1 + 2) * 3) = None::<i32>;
  |               ^^^^^ arbitrary expressions are not allowed in patterns

It's somewhat verbose, but that can be improved in a follow-up.

I wonder if the match arm guard suggestions should be moved to another PR? Tracking issue #129967 will move the guard from the match arm to the pattern, so it might be best to wait for guard_patterns's implementation's PR to be merged before adding guard suggestions.

@rustbot ready

nnethercote · 2024-09-05T21:15:19Z

compiler/rustc_parse/src/parser/pat.rs

+    /// Called by [`Parser::parse_stmt_without_recovery`], used to add statement-aware subdiagnostics to the errors stashed
+    /// by [`Parser::maybe_recover_trailing_expr`].
+    pub(super) fn maybe_augment_stashed_expr_in_pats_with_suggestions(&mut self, stmt: &Stmt) {
+        if self.dcx().has_errors().is_none() {


Can you use has_stashed_diagnostic for a more precise test?

It requires the exact span which I don't have, and there's no

fn has_stashed_diagnostics(self: &DiagCtxtInner) -> bool { !self.stashed_diagnostics.is_empty() }

So it's kinda the best I can do without modifying the stashing API.

compiler/rustc_errors/src/lib.rs

bors · 2024-09-08T01:46:59Z

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

fmease

Tracking issue #129967 will move the guard from the match arm to the pattern, so it might be best to wait for guard_patterns's implementation's PR to be merged before adding guard suggestions.

I see what you're getting at but erm, I'd rather merge this first. This PR is a lot older and will immediately affect diagnostics. We don't know how long it will take for the initial guard-pattern PR to get to a ready state and merged. Also, it shouldn't be that hard to update this code, I think.

I'll bors-approve after a rebase. Thanks a lot for your patience! I took way too long for this

ShE3py · 2024-09-18T21:08:22Z

@rustbot ready

fmease · 2024-09-18T23:46:04Z

@bors r+

bors · 2024-09-18T23:46:06Z

📌 Commit db09345 has been approved by fmease

It is now in the queue for this repository.

bors · 2024-09-19T00:36:36Z

⌛ Testing commit db09345 with merge df7f778...

bors · 2024-09-19T03:28:25Z

☀️ Test successful - checks-actions
Approved by: fmease
Pushing df7f778 to master...

rust-timer · 2024-09-19T04:44:25Z

Finished benchmarking commit (df7f778): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 0.9%, secondary 2.2%)

This 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.

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.9%, 0.9%]	1
Regressions ❌ (secondary)	2.2%	[2.1%, 2.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.9%, 0.9%]	1

Cycles

Results (secondary -2.3%)

This 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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.4%, -2.2%]	4
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 766.835s -> 767.849s (0.13%)
Artifact size: 341.29 MiB -> 341.34 MiB (0.02%)

rustbot assigned fmease Apr 13, 2024

This comment has been minimized.

Sign in to view

ShE3py commented Apr 13, 2024

View reviewed changes

compiler/rustc_errors/src/lib.rs Outdated Show resolved Hide resolved

ShE3py force-pushed the expr-in-pats-2 branch from c82d619 to bf0a1a6 Compare April 25, 2024 01:56

ShE3py mentioned this pull request Apr 26, 2024

Fix substitution parts having a shifted underline in some cases #124370

Merged

ShE3py force-pushed the expr-in-pats-2 branch from a580d10 to a34ced9 Compare April 29, 2024 08:07

ShE3py commented Apr 29, 2024

View reviewed changes

compiler/rustc_parse/src/parser/item.rs Outdated Show resolved Hide resolved

ShE3py force-pushed the expr-in-pats-2 branch from a34ced9 to 5e2c8eb Compare July 1, 2024 18:46

This comment has been minimized.

Sign in to view

ShE3py force-pushed the expr-in-pats-2 branch 4 times, most recently from 1361d1a to 27b59b3 Compare July 7, 2024 12:31

ShE3py marked this pull request as ready for review July 7, 2024 14:04

fmease reviewed Jul 11, 2024

View reviewed changes

compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved

ShE3py force-pushed the expr-in-pats-2 branch from d2ca561 to d4b44ba Compare July 15, 2024 13:46

fmease reviewed Jul 24, 2024

View reviewed changes

ShE3py force-pushed the expr-in-pats-2 branch from d4b44ba to a604d6f Compare July 24, 2024 13:27

This comment has been minimized.

Sign in to view

ShE3py force-pushed the expr-in-pats-2 branch from e6cfede to 47413ca Compare September 5, 2024 15:29

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2024

nnethercote reviewed Sep 5, 2024

View reviewed changes

compiler/rustc_errors/src/lib.rs Outdated Show resolved Hide resolved

fmease mentioned this pull request Sep 14, 2024

Fix Parser::break_up_float's right span #130349

Merged

fmease approved these changes Sep 18, 2024

View reviewed changes

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2024

ShE3py added 2 commits September 18, 2024 20:37

Recover more expressions in patterns

c204721

Add suggestions for expressions in patterns

db09345

ShE3py force-pushed the expr-in-pats-2 branch from 47413ca to db09345 Compare September 18, 2024 19:52

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 18, 2024

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 Sep 18, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 19, 2024

bors merged commit df7f778 into rust-lang:master Sep 19, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Sep 19, 2024

bors mentioned this pull request Sep 19, 2024

Implement Guard Patterns down to THIR #129996

Open

fmease mentioned this pull request Sep 30, 2024

Further improve diagnostics for expressions in pattern position #121697

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further improve diagnostics for expressions in pattern position #123877

Further improve diagnostics for expressions in pattern position #123877

ShE3py commented Apr 13, 2024 •

edited

Loading

This comment has been minimized.

fmease commented Apr 27, 2024

ShE3py commented Apr 29, 2024

bors commented Jun 19, 2024

This comment has been minimized.

fmease left a comment •

edited

Loading

fmease Jul 11, 2024

fmease Jul 11, 2024

fmease Jul 11, 2024

fmease Jul 11, 2024

bors commented Jul 15, 2024

This comment has been minimized.

fmease commented Aug 20, 2024

ShE3py commented Sep 5, 2024

nnethercote Sep 5, 2024

ShE3py Sep 14, 2024

bors commented Sep 8, 2024

fmease left a comment

ShE3py commented Sep 18, 2024

fmease commented Sep 18, 2024

bors commented Sep 18, 2024

bors commented Sep 19, 2024

bors commented Sep 19, 2024

rust-timer commented Sep 19, 2024

Further improve diagnostics for expressions in pattern position #123877

Further improve diagnostics for expressions in pattern position #123877

Conversation

ShE3py commented Apr 13, 2024 • edited Loading

This comment has been minimized.

fmease commented Apr 27, 2024

ShE3py commented Apr 29, 2024

bors commented Jun 19, 2024

This comment has been minimized.

fmease left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jul 15, 2024

This comment has been minimized.

fmease commented Aug 20, 2024

ShE3py commented Sep 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Sep 8, 2024

fmease left a comment

Choose a reason for hiding this comment

ShE3py commented Sep 18, 2024

fmease commented Sep 18, 2024

bors commented Sep 18, 2024

bors commented Sep 19, 2024

bors commented Sep 19, 2024

rust-timer commented Sep 19, 2024

Overall result: no relevant changes - no action needed

ShE3py commented Apr 13, 2024 •

edited

Loading

fmease left a comment •

edited

Loading