Suggest replacing braces for brackets on array-esque invalid block expr

[hkmatsumoto]

Newcomers may write {1, 2, 3} for making arrays, and the current error message is not informative enough to quickly convince them what is needed to fix the error.

This PR implements a diagnostic for this case, and its output looks like this:

error: this code is interpreted as a block expression, not an array
 --> src/lib.rs:1:22
  |
1 |   const FOO: [u8; 3] = {
  |  ______________________^
2 | |     1, 2, 3
3 | | };
  | |_^
  |
  = note: to define an array, one would use square brackets instead of curly braces
help: try using [] instead of {}
  |
1 | const FOO: [u8; 3] = [
2 |     1, 2, 3
3 | ];
  |

Fix #87672

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[lqd]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 82f0454953c87b55491dd1d0a0a8c08aa0a77133 with merge 3337a035d2b47609aaf087adc04a0cbba7e3beb0...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3337a035d2b47609aaf087adc04a0cbba7e3beb0 (3337a035d2b47609aaf087adc04a0cbba7e3beb0)

[Mark-Simulacrum]

@rust-timer build 3337a035d2b47609aaf087adc04a0cbba7e3beb0

[rust-timer]

Queued 3337a035d2b47609aaf087adc04a0cbba7e3beb0 with parent d488de8, future comparison URL.

[rust-timer]

Finished benchmarking try commit (3337a035d2b47609aaf087adc04a0cbba7e3beb0): comparison url.

Summary: This benchmark run did not return any significant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

[estebank]

I have a somewhat different approach that might be able to make some of this code easier to reason about.

Instead of using look_ahead for cases where we kind of need to parse things with non-deterministic lookahead, I do let snapthot = self.clone() on the Parser and try to successfully parse what the "fake" syntax would be. If that succeeds, we know that we can provide an accurate suggestion, if that doesn't, we cancel the error and rollback the parser to the previous state to emit the "normal" error we currently see and let the existing error recovery machinery take its course. With this approach, it should be possible to emit a suggestion on a case like {0, if 1 < 2 {1} else {1}, 2, 3}. Because cloning is somewhat expensive (but not terrible, from what I've seen), we want to avoid it on the "happy path" of well-formed code, but you can use limited look_ahead to see if it could be this case, like doing self.look_ahead(2, |t| t.kind == &Comma) or something.

We could even try to keep information on the Parser for "we are in a const assignment, of array type" that we check here in maybe_suggest_brackets_instead_of_braces, and if it is not the case, we exit early, like we do for things coming from a macro.

What do you think? (Sorry for the delay in reviewing)

[hkmatsumoto]

It's very smart of you to come up with the approach :)
What's nice is that by cloning the parser the codebase will be significantly simplified while the suggestion keeps working almost as-is except in some cases like {if 1 < 2 {1} else {1}, 2, 3}, but that's compromisable. I assume it's unlikely to initialize an array with complex expressions.

[estebank]

Thank you, you're too kind.

Yes, I agree that this gets 80% there for 20% of the effort. If we were relying on this for the actual language and not just for error recovery, the story would be different.

[estebank]

Let's add a test as well :)

[estebank]

@bors r+

[bors]

📌 Commit 21eff8f has been approved by estebank

[bors]

⌛ Testing commit 21eff8f with merge e7958d3...

[bors]

☀️ Test successful - checks-actions
Approved by: estebank
Pushing e7958d3 to master...