Skip to content

Recover and suggest to use ; to construct array type #143905

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

Merged
merged 2 commits into from
Jul 15, 2025

Conversation

xizheyin
Copy link
Contributor

@xizheyin xizheyin commented Jul 13, 2025

Fixes #143828

r? compiler

@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. labels Jul 13, 2025
Comment on lines 15 to 18
help: use `=` if you meant to assign
|
LL - let b: [i32, 5];
LL + let b = [i32, 5];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This suggestion is wrong, but it have nothing with this PR. Because the old diagnostic has the same help message.

Old Diagnostic:

error: expected one of `!`, `(`, `+`, `::`, `;`, `<`, or `]`, found `,`
 --> src/main.rs:3:16
  |
3 |     let b: [i32, 5];
  |          -     ^ expected one of 7 possible tokens
  |          |
  |          while parsing the type for `b`
  |          help: use `=` if you meant to assign

@compiler-errors
Copy link
Member

r? compiler-errors

@rustbot author

@rustbot rustbot assigned compiler-errors and unassigned oli-obk Jul 13, 2025
@rustbot 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 Jul 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 13, 2025

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

@xizheyin
Copy link
Contributor Author

Also, why can't we emit the error and then return TyKind::Array, so we can recover successfully rather than propagating an error?

Yeah, I have tried to recover it to TyKind::Array, but It emit a regression. In this test, it will emit error: expected ; or ], found , immidiately, which is wrong. It just want call parse_ty_common for Some(x), while in this case, it parse it into Array and give a wrong suggestion.

https://github.com/rust-lang/rust/blob/master/tests/ui/issues/issue-50571.rs
https://github.com/rust-lang/rust/blob/master/tests/ui/issues/issue-50571.stderr

@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 Jul 14, 2025
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

I personally think it's fine to regress #50571, since it's an issue that only has to do with edition 2015 and is a rare enough.

I'd still like to see a better recovery that actually ends up returning Ok(TyKind::Array(...)).

@rustbot 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 Jul 14, 2025
@xizheyin xizheyin force-pushed the 143828 branch 2 times, most recently from 4c2fd87 to edd7377 Compare July 14, 2025 06:41
Copy link
Contributor Author

@xizheyin xizheyin left a comment

Choose a reason for hiding this comment

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

@rustbot ready

Comment on lines 8 to 11
help: you might have meant to use `;` as the separator
|
LL | type v = [isize ;* 3];
| +
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This regression is somewhat werid, because parse_expr_anon_const take it as a deref of 3. That maybe reasonable for

const L = &3;
type V = [isize * L];

After applying the suggestion, it is correct.

type V = [isize ;* L];

But IMO, it does not matter in this * case, because after applying the suggestion, compiler will report something like you can not deref a integer, which is reasonable.

Copy link
Member

Choose a reason for hiding this comment

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

We could perhaps special case both , and * as separators here?

Comment on lines +38 to +48
error: expected `;` or `]`, found `5`
--> $DIR/array-type-no-semi.rs:15:17
|
LL | let e: [i32 5];
| ^ expected `;` or `]`
|
= note: you might have meant to write a slice or array type
help: you might have meant to use `;` as the separator
|
LL | let e: [i32 ;5];
| +
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the new case like [i32 5].

@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 Jul 14, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 14, 2025

There are changes to the tidy tool.

cc @jieyouxu

@xizheyin xizheyin changed the title Suggest use ; to construct array type when other token exists Recover and suggest to use ; to construct array type Jul 14, 2025
Comment on lines 8 to 11
help: you might have meant to use `;` as the separator
|
LL | type v = [isize ;* 3];
| +
Copy link
Member

Choose a reason for hiding this comment

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

We could perhaps special case both , and * as separators here?

Comment on lines 609 to 614
let length = match self.parse_expr_anon_const() {
Ok(length) => length,
Err(e) => {
e.cancel();
self.bump();
match self.parse_expr_anon_const() {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of parsing once then parsing again, could you do:

let suggestion_span = if self.eat(comma) || self.eat(asterisk) {
    // Consume common erroneous separators.
    self.prev_token.span
} else {
    self.token.span.shrink_to_lo()
};

/// then parse the length.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should other separators be considered, such as :?

Copy link
Member

Choose a reason for hiding this comment

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

I think , and * are fine for now

@xizheyin
Copy link
Contributor Author

Yeah, I have instead specially judge , and *.

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 15, 2025

📌 Commit ed88af2 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jul 15, 2025
bors added a commit that referenced this pull request Jul 15, 2025
Rollup of 13 pull requests

Successful merges:

 - #142301 (tests: Fix duplicated-path-in-error fail with musl)
 - #143630 (Drop `./x suggest`)
 - #143736 (Give all bytes of TypeId provenance)
 - #143752 (Don't panic if WASI_SDK_PATH not set when detecting compiler)
 - #143837 (Adjust `run_make_support::symbols` helpers)
 - #143878 (Port `#[pointee]` to the new attribute parsing infrastructure)
 - #143905 (Recover and suggest to use `;` to construct array type)
 - #143907 (core: make `str::split_at_unchecked()` inline)
 - #143910 (Add experimental `backtrace-trace-only` std feature)
 - #143927 (Preserve constness in trait objects up to hir ty lowering)
 - #143935 (rustc_type_ir/walk: move docstring to `TypeWalker` itself)
 - #143938 (Update books)
 - #143941 (update `cfg_select!` documentation)

Failed merges:

 - #143926 (Remove deprecated fields in bootstrap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 281990e into rust-lang:master Jul 15, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 15, 2025
rust-timer added a commit that referenced this pull request Jul 15, 2025
Rollup merge of #143905 - xizheyin:143828, r=compiler-errors

Recover and suggest to use `;` to construct array type

Fixes #143828

r? compiler
@xizheyin xizheyin deleted the 143828 branch July 15, 2025 18:00
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jul 17, 2025
Rollup of 13 pull requests

Successful merges:

 - rust-lang/rust#142301 (tests: Fix duplicated-path-in-error fail with musl)
 - rust-lang/rust#143630 (Drop `./x suggest`)
 - rust-lang/rust#143736 (Give all bytes of TypeId provenance)
 - rust-lang/rust#143752 (Don't panic if WASI_SDK_PATH not set when detecting compiler)
 - rust-lang/rust#143837 (Adjust `run_make_support::symbols` helpers)
 - rust-lang/rust#143878 (Port `#[pointee]` to the new attribute parsing infrastructure)
 - rust-lang/rust#143905 (Recover and suggest to use `;` to construct array type)
 - rust-lang/rust#143907 (core: make `str::split_at_unchecked()` inline)
 - rust-lang/rust#143910 (Add experimental `backtrace-trace-only` std feature)
 - rust-lang/rust#143927 (Preserve constness in trait objects up to hir ty lowering)
 - rust-lang/rust#143935 (rustc_type_ir/walk: move docstring to `TypeWalker` itself)
 - rust-lang/rust#143938 (Update books)
 - rust-lang/rust#143941 (update `cfg_select!` documentation)

Failed merges:

 - rust-lang/rust#143926 (Remove deprecated fields in bootstrap)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tidy Area: The tidy tool S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

Writing array type with , instead of ; could have a friendlier error
7 participants