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

"help: } may belong here" no longer reported #108608

Open
lukas-code opened this issue Mar 1, 2023 · 5 comments
Open

"help: } may belong here" no longer reported #108608

lukas-code opened this issue Mar 1, 2023 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-bug Category: This is a bug. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lukas-code
Copy link
Member

After #108297, the help message for inserting closing delims is no longer emitted. (cc @chenyukang)

Code

I tried this code:

use std::{mem, ptr;

old output:

error: this file contains an unclosed delimiter
 --> src/main.rs:1:20
  |
1 | use std::{mem, ptr;
  |          -         ^
  |          |
  |          unclosed delimiter

error: expected one of `,`, `::`, `as`, or `}`, found `;`
 --> src/main.rs:1:10
  |
1 | use std::{mem, ptr;
  |          ^        ^
  |          |        |
  |          |        expected one of `,`, `::`, `as`, or `}`
  |          |        help: `}` may belong here
  |          unclosed delimiter

error: expected item, found `}`
 --> src/main.rs:1:20
  |
1 | use std::{mem, ptr;
  |                    ^ expected item

new output

error: this file contains an unclosed delimiter
 --> src/main.rs:1:20
  |
1 | use std::{mem, ptr;
  |          -         ^
  |          |
  |          unclosed delimiter

While noisy, the old output contains a concrete fix (} may belong here) and the new one doesn't.

Version it worked on

It most recently worked on: rustc 1.69.0-nightly (31f858d 2023-02-28)

Version with regression

master at 64165aa, which will be in nightly 2023-03-01

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged +A-parser +A-diagnostics

@lukas-code lukas-code added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Mar 1, 2023
@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-untriaged Untriaged performance or correctness regression. labels Mar 1, 2023
@apiraino
Copy link
Contributor

apiraino commented Mar 1, 2023

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-low

@rustbot rustbot added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 1, 2023
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 1, 2023
@chenyukang
Copy link
Member

chenyukang commented Mar 1, 2023

Yes, it is a know issue for last PR(we have UI test for it).
It's a tradeoff, I made #108297 based on these considerations:

  • unclosed/mismatch delimiter may introduce totally incorrect tokenstream, which will make the parser understand the code totally wrong and give a lot of noisy diagnostics.
  • there may be multiple fixes for an unclosed/mismatched delimiter issue, we don't know which one is the best
  • the first diagnostics is most important in this scenario.

Given an unclosed delimiter issue, for example, this code:

use std::{mem;

fn main() {
    assert_eq!(4, mem::size_of::<i32>());
}

Stable version will also give the help to add another }:

error: expected one of `,`, `::`, `as`, or `}`, found `;`
 --> src/main.rs:1:10
  |
1 | use std::{mem;
  |          ^   ^
  |          |   |
  |          |   expected one of `,`, `::`, `as`, or `}`
  |          |   help: `}` may belong here
  |          unclosed delimiter

while the better way to fix it is removing the {.

And the output from your description is much more confusing, since user didn't input a }:

error: expected item, found `}`
 --> src/main.rs:1:20
  |
1 | use std::{mem, ptr;
  |                    ^ expected item

@chenyukang
Copy link
Member

Another case that mismatch delimiter can not be handled well:

fn bar() -> [u8; 2] {
    foo()
    [1, 3) 
}

fn main() {}

The stable compiler output:

error: expected one of `.`, `?`, `]`, or an operator, found `,`
 --> src/main.rs:4:5
  |
4 |     [1, 3) 
  |     ^ ^ help: `]` may belong here
  |     |
  |     unclosed delimiter

@workingjubilee workingjubilee removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Mar 3, 2023
@workingjubilee
Copy link
Member

workingjubilee commented Mar 3, 2023

Our diagnostics are unstable. Changes in them are not automatically considered regressions (at least, not without some other detail). As this was an intentional change which yields an improvement in other ways, I am inclined to class this as a non-issue.

@Dylan-DPC
Copy link
Member

Closing this as it was an intentional change and cannot be considered a regression.

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 C-bug Category: This is a bug. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants