Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554

xizheyin · 2025-03-16T07:48:32Z

rustbot · 2025-03-16T07:48:36Z

rustbot has assigned @compiler-errors.
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

compiler-errors · 2025-03-21T16:41:23Z

r? compiler

chenyukang · 2025-04-07T06:34:07Z

I changed this part last year, will review this PR later.

r? @chenyukang

tests/ui/parser/issues/issue-67377-invalid-syntax-in-enum-discriminant.stderr

tests/ui/parser/issues/issue-68987-unmatch-issue-2.stderr

tests/ui/parser/macro-mismatched-delim-paren-brace.stderr

tests/ui/parser/issues/issue-68987-unmatch-issue-3.stderr

chenyukang · 2025-04-11T04:24:40Z

I think this solution introduces some kind of regression on other scenarios(such as the cases I commented).

Maybe we should consider this as a special corner case and provide a special diagnostic for it, it is the pattern that one delimiter is missing the openning delimiter(it's in the inner part), except for this everything is ok, so we should suggest something like "missing openning delimiter ....".

such as:

{ ... {. ) ... }...} missing a inner (
{ ... [ ], ] ...} missing a inner [

chenyukang · 2025-04-11T04:25:05Z

@rustbot author

rustbot · 2025-04-11T04:25:08Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

xizheyin · 2025-04-11T05:11:09Z

Thanks, I will revise it soon!

xizheyin · 2025-04-12T10:48:15Z

This seems like quite a lot of changes, I'll comment in the code to make it easier to REVIEW.

In the first commit, I added the test.

In the second commit I did some clean. I changed a variable name, the original name was inaccurate, I used open_delimiters instead of open_braces for identifying blocks, one should compare spans, not just brace'{'.

In the third commit, I make a distinction between the different delimiters before triggering a unexpected closing delimiter error. But I found that missing open delim note is what triggers in unexpected closing delimiter error.
Therefore, to emit missing open delim note, I also moved missing open delim note to
mismatched closing delimiter error, which can be emitted without delaying to
unexpected closing delimiter error.
And I moved function make_unclosed_delims_error
to src/lexer/diagnostics.rs for clean. And some other clean.

compiler/rustc_parse/src/lexer/diagnostics.rs

compiler/rustc_parse/src/lexer/tokentrees.rs

tests/ui/parser/issues/issue-68987-unmatch-issue-2.stderr

tests/ui/parser/issues/issue-68987-unmatch-issue-3.stderr

tests/ui/parser/deli-ident-issue-2.stderr

xizheyin · 2025-04-15T08:13:44Z

I've combined them into one line so it's more aesthetically pleasing.

mismatched closing delimiter, may missing open `(`

Since the mismatched closing delimiter note and the unclosed delimiter note are related, it may be the better to keep it.

chenyukang · 2025-04-21T15:56:44Z

tests/ui/attributes/z-crate-attr/unbalanced-paren.stderr

@@ -2,7 +2,7 @@ error: mismatched closing delimiter: `]`
  --> <crate attribute>:1:4
   |
 LL | #![(]
-   |   -^^ mismatched closing delimiter
+   |   -^^ mismatched closing delimiter, may missing open `[`
   |   ||


seems we are suggestting writing code like this?

#![([]

seems not helpful.

chenyukang · 2025-04-21T16:02:28Z

tests/ui/macros/issue-102878.stderr

@@ -2,7 +2,7 @@ error: mismatched closing delimiter: `)`
  --> $DIR/issue-102878.rs:1:35
   |
 LL | macro_rules!test{($l:expr,$_:r)=>({const:y y)}
-   |                                  -^         ^ mismatched closing delimiter
+   |                                  -^         ^ mismatched closing delimiter, may missing open `(`
   |                                  ||


this is worse too, we are not sure we should replace the left { to ( or replace the right ) to }.

so, the better way is do not suggest any direction.

I think we should only suggest missing a open(or closing) delimiter in the scenario that if we added a open(or closing) delimiter, there will be no error.

chenyukang · 2025-04-21T16:03:28Z

tests/ui/parser/deli-ident-issue-2.stderr

@@ -4,16 +4,7 @@ error: mismatched closing delimiter: `]`
 LL |     if 1 < 2 {
   |              ^ unclosed delimiter
 LL |         let _a = vec!];
-   |                      ^ mismatched closing delimiter
+   |                      ^ mismatched closing delimiter, may missing open `[`


for example, this is the scenario we should report missing open [, because except for this ] is lefted unmistached, all the other outer delimiter are matched correctly.

chenyukang · 2025-04-21T16:05:35Z

tests/ui/parser/do-not-suggest-semicolon-before-array.stderr

@@ -2,7 +2,7 @@ error: mismatched closing delimiter: `)`
  --> $DIR/do-not-suggest-semicolon-before-array.rs:5:5
   |
 LL |     [1, 3)
-   |     ^    ^ mismatched closing delimiter
+   |     ^    ^ mismatched closing delimiter, may missing open `(`


this is similar, it maybe:

[1, 3] or (1, 3), we can not determine which is right at this point.

This example should be similar to Result. In fact, it is difficult for a compiler to determine which one is correct. Do you have any suggestions for implementation? :)

chenyukang · 2025-04-21T16:07:27Z

tests/ui/parser/issues/issue-68987-unmatch-issue-2.stderr

@@ -4,16 +4,7 @@ error: mismatched closing delimiter: `)`
 LL | async fn obstest() -> Result<> {
   |                                ^ unclosed delimiter
 LL |     let obs_connect = || -> Result<(), MyError) {
-   |                                               ^ mismatched closing delimiter
+   |                                               ^ mismatched closing delimiter, may missing open `(`



this suggesting:

Result((), MyError)

while right code should:

Result<(), MyError>

For example, for this example, how do we know whether it should be) or > through the oracle? It seems easy for humans to understand, but it is not easy in the compiler.

yes, compiler can not suggest anything for this, so we should not add the new label "may missing open (". just keep the current ^ mismatched closing delimiter is ok.

I see, that is to say, the compiler should only give advice when it is sure, otherwise it just needs to report an error.

xizheyin · 2025-04-21T16:40:34Z

I don't have a good way to judge whether adding the open delimiter is a better way. As in the previous implementation, there will always be some false positives.

If we have to make a balance, should we make suggestions as comprehensively as possible, or would we rather not make suggestions and minimize false positives?

Maybe I need to refactor this code more.

chenyukang · 2025-04-21T16:43:54Z

tests/ui/parser/issues/unnessary-error-issue-138401.stderr

+LL | pub fn foo(x: i64) -> i64 {
+   |                           ^ unclosed delimiter
+LL |     x.abs)
+   |          ^ mismatched closing delimiter, may missing open `(`


it's better to suggest like this:

missing open `(` for this delimiter

as the origin issue reported wrote in #138401

chenyukang · 2025-04-21T16:45:39Z

compiler/rustc_parse/src/lexer/tokentrees.rs

@@ -23,7 +24,7 @@ impl<'psess, 'src> Lexer<'psess, 'src> {
                    // Invisible delimiters cannot occur here because `TokenTreesReader` parses
                    // code directly from strings, with no macro expansion involved.
                    debug_assert!(!matches!(delim, Delimiter::Invisible(_)));
-                    buf.push(match self.lex_token_tree_open_delim(delim) {
+                    buf.push(match self.lex_token_tree_open_delim(delim, self.token.span) {


I guess this may have a performance effect since it's on the normal code path.

Here, after the modification is confirmed, perhaps a time robot can be used to test it.

chenyukang · 2025-04-21T16:53:13Z

I have a look at the current code, current output is:

error: mismatched closing delimiter: `)`
 --> ...\test.rs:1:27
  |
1 | pub fn foo(x: i64) -> i64 {
  |                           ^ unclosed delimiter
2 |     x.abs)
  |          ^ mismatched closing delimiter

error: unexpected closing delimiter: `}`
 --> ...\test.rs:3:1
  |
2 |     x.abs)
  |          - missing open `(` for this delimiter
3 | }
  | ^ unexpected closing delimiter

error: aborting due to 2 previous errors

The first diagnostic message is reported at here(you moved it into lexer, it is ok):
https://github.com/chenyukang/rust/blob/c8f94230282a8e8c1148f3e657f0199aad909228/compiler/rustc_parse/src/parser/mod.rs#L1766-L1785

The second diagnostic is reported at here:
https://github.com/chenyukang/rust/blob/c8f94230282a8e8c1148f3e657f0199aad909228/compiler/rustc_parse/src/lexer/diagnostics.rs#L61

To resolve the issue in #138401, can we just remove the first diagnostic if we could report the second one? or at least reorder these two diagnostics, which could also reduce the confusion.

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

…when push matching_block_span Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

…elimiter To emit missing open delim note, I also moved `missing open delim` note to `mismatched closing delimiter` error, which can emit without delaying to `unexpected closing delimiter` error. And I moved `make_unclosed_delims_error` to src/lexer/diagnostics.rs for clean. And some other clean. Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin · 2025-04-22T06:48:40Z

This part of the code conflicted with master, so I submitted the PR #140147 first to do some cleanup, otherwise the code for this PR would have been rather messy. @chenyukang

rustbot assigned compiler-errors Mar 16, 2025

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 Mar 16, 2025

rustbot assigned estebank and unassigned compiler-errors Mar 21, 2025

rustbot assigned chenyukang and unassigned estebank Apr 7, 2025

chenyukang reviewed Apr 11, 2025

View reviewed changes

tests/ui/parser/issues/issue-67377-invalid-syntax-in-enum-discriminant.stderr Outdated Show resolved Hide resolved

chenyukang requested changes Apr 11, 2025

View reviewed changes

tests/ui/parser/issues/issue-68987-unmatch-issue-2.stderr Show resolved Hide resolved

rustbot 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 Apr 11, 2025

chenyukang reviewed Apr 11, 2025

View reviewed changes

tests/ui/parser/macro-mismatched-delim-paren-brace.stderr Outdated Show resolved Hide resolved

chenyukang reviewed Apr 11, 2025

View reviewed changes

tests/ui/parser/issues/issue-68987-unmatch-issue-3.stderr Show resolved Hide resolved

xizheyin force-pushed the issue-138401 branch from 0c098ca to b44d336 Compare April 12, 2025 10:35

xizheyin changed the title ~~[Lexer] Remove spurious unexpected delimiter error by matching remain…~~ Distinguish delim kind to decide whether to emit unexpected closing delimiter Apr 12, 2025

xizheyin commented Apr 12, 2025

View reviewed changes

xizheyin requested a review from chenyukang April 12, 2025 11:02

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 Apr 12, 2025

chenyukang reviewed Apr 15, 2025

View reviewed changes

tests/ui/parser/deli-ident-issue-2.stderr Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

xizheyin force-pushed the issue-138401 branch from 7d08a64 to d4f88b5 Compare April 15, 2025 09:53

This comment has been minimized.

Sign in to view

xizheyin force-pushed the issue-138401 branch from d4f88b5 to 57cc78b Compare April 15, 2025 11:56

chenyukang reviewed Apr 21, 2025

View reviewed changes

xizheyin added 4 commits April 22, 2025 12:22

Add ui test tests/ui/parser/issues/unnessary-error-issue-138401.rs

17b15ac

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

clean: rename open_brace to open_delimiters, and add span constraint …

53970cb

…when push matching_block_span Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

combine mismatched closing and missing open into one line

4d7b7ed

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>

xizheyin force-pushed the issue-138401 branch from 57cc78b to 4d7b7ed Compare April 22, 2025 05:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554

Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554

xizheyin commented Mar 16, 2025 •

edited

Loading

rustbot commented Mar 16, 2025

compiler-errors commented Mar 21, 2025

chenyukang commented Apr 7, 2025

chenyukang commented Apr 11, 2025

chenyukang commented Apr 11, 2025

rustbot commented Apr 11, 2025

xizheyin commented Apr 11, 2025

xizheyin commented Apr 12, 2025

xizheyin commented Apr 15, 2025

This comment has been minimized.

This comment has been minimized.

chenyukang Apr 21, 2025

chenyukang Apr 21, 2025 •

edited

Loading

chenyukang Apr 21, 2025

chenyukang Apr 21, 2025

xizheyin Apr 21, 2025

chenyukang Apr 21, 2025

xizheyin Apr 21, 2025

chenyukang Apr 21, 2025

xizheyin Apr 21, 2025

xizheyin commented Apr 21, 2025

chenyukang Apr 21, 2025

chenyukang Apr 21, 2025

xizheyin Apr 21, 2025

chenyukang commented Apr 21, 2025

xizheyin commented Apr 22, 2025 •

edited

Loading

Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554

Are you sure you want to change the base?

Distinguish delim kind to decide whether to emit unexpected closing delimiter #138554

Conversation

xizheyin commented Mar 16, 2025 • edited Loading

rustbot commented Mar 16, 2025

compiler-errors commented Mar 21, 2025

chenyukang commented Apr 7, 2025

chenyukang commented Apr 11, 2025

chenyukang commented Apr 11, 2025

rustbot commented Apr 11, 2025

xizheyin commented Apr 11, 2025

xizheyin commented Apr 12, 2025

xizheyin commented Apr 15, 2025

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

chenyukang Apr 21, 2025 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xizheyin commented Apr 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenyukang commented Apr 21, 2025

xizheyin commented Apr 22, 2025 • edited Loading

xizheyin commented Mar 16, 2025 •

edited

Loading

chenyukang Apr 21, 2025 •

edited

Loading

xizheyin commented Apr 22, 2025 •

edited

Loading