Skip to content
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

Recover parentheses in range patterns #119397

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

ShE3py
Copy link
Contributor

@ShE3py ShE3py commented Dec 28, 2023

Before:

match n {
    (0).. => (),
    _ => ()
}
error: expected one of `=>`, `if`, or `|`, found `..`
 --> src/lib.rs:3:12
  |
3 |         (0).. => (),
  |            ^^ expected one of `=>`, `if`, or `|`

After:

error: range pattern bounds cannot have parentheses
 --> main.rs:3:5
  |
3 |     (0).. => (),
  |     ^ ^
  |
help: remove these parentheses
  |
3 -     (0).. => (),
3 +     0.. => (),
  |

This sets the groundwork for #118625, which will extend the recovery to expressions like (0 + 1).. where users may tend to add parentheses to avoid dealing with precedence.


@rustbot label +A-parser +A-patterns +A-diagnostics

@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot 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. A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-patterns Relating to patterns and pattern matching labels Dec 28, 2023
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this needs .may_recover() checks in some places not to regress code like:

macro_rules! m {
    ($pat:pat) => {};
    (($l:literal)..) => {};
}

fn main() {
    m!((0)..);
}

The code above now fails to compile or am I mistaken?

@rust-log-analyzer

This comment has been minimized.

compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/errors.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/pat.rs Outdated Show resolved Hide resolved
@fmease
Copy link
Member

fmease commented Dec 30, 2023

r? fmease

@rustbot rustbot assigned fmease and unassigned TaKO8Ki Dec 30, 2023
@fmease 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 Dec 30, 2023
ShE3py added a commit to ShE3py/rust that referenced this pull request Dec 30, 2023
Co-authored-by: León Orell Valerian Liehr <[email protected]>
ShE3py added a commit to ShE3py/rust that referenced this pull request Dec 30, 2023
Co-authored-by: León Orell Valerian Liehr <[email protected]>
ShE3py added a commit to ShE3py/rust that referenced this pull request Dec 31, 2023
@ShE3py
Copy link
Contributor Author

ShE3py commented Dec 31, 2023

@rustbot ready

@rustbot 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 Dec 31, 2023
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for applying my suggestions!

I've had a think about #119397 (comment) again and while that diagnostic (expected `)`, found `+`) isn't ideal, you plan on fixing that soon anyway . Now, the latest version of your PR contains two ad hoc diagnostics (unexpected token: `(` ) which aren't ideal either.

So let's go back to bailing out early with ? if we fail to parse bound in parse_pat_range_end and to expect'ing ) instead of eat_noexpect'ing to eliminate both occurrences of struct_span_err.

After, that this PR is good to go! Sorry for taking a while to respond. I'm excited to see you improving the parse errors for patterns! ❤️

Co-authored-by: León Orell Valerian Liehr <[email protected]>
@fmease
Copy link
Member

fmease commented Jan 3, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 3, 2024

📌 Commit 4e0badd has been approved by fmease

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 3, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors 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 Jan 3, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 3, 2024
…=fmease

Recover parentheses in range patterns

Before:
```rs
match n {
    (0).. => (),
    _ => ()
}
```
```
error: expected one of `=>`, `if`, or `|`, found `..`
 --> src/lib.rs:3:12
  |
3 |         (0).. => (),
  |            ^^ expected one of `=>`, `if`, or `|`
  ```
 After:
```
error: range pattern bounds cannot have parentheses
 --> main.rs:3:5
  |
3 |     (0).. => (),
  |     ^ ^
  |
help: remove these parentheses
  |
3 -     (0).. => (),
3 +     0.. => (),
  |
```

This sets the groundwork for rust-lang#118625, which will extend the recovery to expressions like `(0 + 1)..` where users may tend to add parentheses to avoid dealing with precedence.

---

`@rustbot` label +A-parser +A-patterns +A-diagnostics
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119184 (Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`)
 - rust-lang#119325 (custom mir: make it clear what the return block is)
 - rust-lang#119391 (Use Result::flatten in catch_with_exit_code)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119414 (bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119527 (don't reexport atomic::ordering via rustc_data_structures, use std import)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)

r? `@ghost`
`@rustbot` modify labels: rollup
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 4, 2024
…=fmease

Recover parentheses in range patterns

Before:
```rs
match n {
    (0).. => (),
    _ => ()
}
```
```
error: expected one of `=>`, `if`, or `|`, found `..`
 --> src/lib.rs:3:12
  |
3 |         (0).. => (),
  |            ^^ expected one of `=>`, `if`, or `|`
  ```
 After:
```
error: range pattern bounds cannot have parentheses
 --> main.rs:3:5
  |
3 |     (0).. => (),
  |     ^ ^
  |
help: remove these parentheses
  |
3 -     (0).. => (),
3 +     0.. => (),
  |
```

This sets the groundwork for rust-lang#118625, which will extend the recovery to expressions like `(0 + 1)..` where users may tend to add parentheses to avoid dealing with precedence.

---

``@rustbot`` label +A-parser +A-patterns +A-diagnostics
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…mpiler-errors

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118521 (Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag)
 - rust-lang#119026 (std::net::bind using -1 for openbsd which in turn sets it to somaxconn.)
 - rust-lang#119195 (Make named_asm_labels lint not trigger on unicode and trigger on format args)
 - rust-lang#119204 (macro_rules: Less hacky heuristic for using `tt` metavariable spans)
 - rust-lang#119362 (Make `derive(Trait)` suggestion more accurate)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119414 (bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)
 - rust-lang#119555 (Add codegen test for RVO on MaybeUninit)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…mpiler-errors

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118521 (Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag)
 - rust-lang#119026 (std::net::bind using -1 for openbsd which in turn sets it to somaxconn.)
 - rust-lang#119195 (Make named_asm_labels lint not trigger on unicode and trigger on format args)
 - rust-lang#119204 (macro_rules: Less hacky heuristic for using `tt` metavariable spans)
 - rust-lang#119362 (Make `derive(Trait)` suggestion more accurate)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119414 (bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)
 - rust-lang#119555 (Add codegen test for RVO on MaybeUninit)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118521 (Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag)
 - rust-lang#119026 (std::net::bind using -1 for openbsd which in turn sets it to somaxconn.)
 - rust-lang#119195 (Make named_asm_labels lint not trigger on unicode and trigger on format args)
 - rust-lang#119204 (macro_rules: Less hacky heuristic for using `tt` metavariable spans)
 - rust-lang#119362 (Make `derive(Trait)` suggestion more accurate)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119539 (Fix typos)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)
 - rust-lang#119555 (Add codegen test for RVO on MaybeUninit)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118521 (Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag)
 - rust-lang#119026 (std::net::bind using -1 for openbsd which in turn sets it to somaxconn.)
 - rust-lang#119195 (Make named_asm_labels lint not trigger on unicode and trigger on format args)
 - rust-lang#119204 (macro_rules: Less hacky heuristic for using `tt` metavariable spans)
 - rust-lang#119362 (Make `derive(Trait)` suggestion more accurate)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119539 (Fix typos)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)
 - rust-lang#119555 (Add codegen test for RVO on MaybeUninit)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#118521 (Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag)
 - rust-lang#119026 (std::net::bind using -1 for openbsd which in turn sets it to somaxconn.)
 - rust-lang#119195 (Make named_asm_labels lint not trigger on unicode and trigger on format args)
 - rust-lang#119204 (macro_rules: Less hacky heuristic for using `tt` metavariable spans)
 - rust-lang#119362 (Make `derive(Trait)` suggestion more accurate)
 - rust-lang#119397 (Recover parentheses in range patterns)
 - rust-lang#119417 (Uplift some miscellaneous coroutine-specific machinery into `check_closure`)
 - rust-lang#119539 (Fix typos)
 - rust-lang#119540 (Don't synthesize host effect args inside trait object types)
 - rust-lang#119555 (Add codegen test for RVO on MaybeUninit)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3325ba6 into rust-lang:master Jan 4, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024
Rollup merge of rust-lang#119397 - ShE3py:pat-range-paren-recovery, r=fmease

Recover parentheses in range patterns

Before:
```rs
match n {
    (0).. => (),
    _ => ()
}
```
```
error: expected one of `=>`, `if`, or `|`, found `..`
 --> src/lib.rs:3:12
  |
3 |         (0).. => (),
  |            ^^ expected one of `=>`, `if`, or `|`
  ```
 After:
```
error: range pattern bounds cannot have parentheses
 --> main.rs:3:5
  |
3 |     (0).. => (),
  |     ^ ^
  |
help: remove these parentheses
  |
3 -     (0).. => (),
3 +     0.. => (),
  |
```

This sets the groundwork for rust-lang#118625, which will extend the recovery to expressions like `(0 + 1)..` where users may tend to add parentheses to avoid dealing with precedence.

---

```@rustbot``` label +A-parser +A-patterns +A-diagnostics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST A-patterns Relating to patterns and pattern matching S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants