Suggest adding missing braces in const block pattern

[camelid]

Fixes#78168.

Previously it would only suggest wrapping the code in braces in regular
expressions; now it does it in patterns too.

r? @spastorino

[spastorino]

I think @petrochenkov and others wanted to avoid eating the keyword before checking if it's really an inline const. My first reaction was like ... but we're eating Unsafe right above. I'm not sure if there's another way to solve this problem and we could still rely on checking or if we just need to eat the keyword.

Anyway, would leave this to @petrochenkov

[eddyb]

The difference is that unsafe just modifies the block expression, whereas const needs to create the ExprKind::Const - but maybe it doesn't have to be too complicated shrug.

[spastorino]

Another thought, I've 2 PRs open for inline consts that changes in some way the code you're also touching. It may be good to land those first and base your work on those.

[camelid]

Yeah, I expected to have merge conflicts :)

Weirdly, GitHub is reporting merge conflicts but bors is not. I think this issue has happened before; I’ll let T-infra take a look before I fix the conflicts so they can perhaps figure out the cause.

[camelid]

Rebased.

[spastorino]

This has regressed a bit now.
cc @oli-obk

[camelid]

The thing is that this:

letconst{"foo"} = "foo";

is a totally valid statement (although it ICEs; see #78174). Perhaps we could give the old warning if it's just const and then = immediately after?

[spastorino]

I see, but, what do you expect that code to do?.
I'd expect an error anyway something like error[E0005]: refutable pattern in local binding: ``&_`` not covered.

[camelid]

It's valid syntactically, not semantically. (And either way it shouldn't ICE.)

And yes, the error you suggested is correct; it's the one that occurs for let "foo" = "foo";: error[E0005]: refutable pattern in local binding: `&_` not covered

[spastorino]

exactly, 👍

[spastorino]

Now both of my follow up PRs are in and you would need to rebase again :). I think this is fine modulo the eat_keyword part, which to be honest I'm not sure. I'm not sure if there's a way to avoid it, my first solution used it too and then after some reviews I got rid of it.

Anyway, this is fine by me but would leave this up to r? @petrochenkov

[petrochenkov]

@camelid
Could you rebase this over #78116? There were some significant changes in the same area in that PR.

[petrochenkov]

@camelid
I have a suggestion for a different approach for const in expressions.
In expressions we have a well established precedent for general-purpose unary operators, including keyword-based ones like box.

So I suggest to parse const expr in expressions exactly like a unary operator box expr, but then report a non-fatal error if expr is not actually a block.
The only difference would be an extra check at statement start to avoid interpreting const IDENT: ... or const _: ... items as const expressions.

In patterns we have the unary box operator too, so const recovery can potentially work in the same way as in expressions (parse const PAT, error if PAT is not a block), but I'm not sure about interactions with range patterns, they may require special cases.

[camelid]

I'm hoping to get to this at some point soon, I just need to understand this part of the codebase better.

[petrochenkov]

This can be split into two parts - one for expressions (which should be pretty much a copypaste of what box does), and another for patterns which may require some more thought.

[camelid]

Starting with a rebase.

[camelid]

I have a suggestion for a different approach for const in expressions.
In expressions we have a well established precedent for general-purpose unary operators, including keyword-based ones like box.

So I suggest to parse const expr in expressions exactly like a unary operator box expr, but then report a non-fatal error if expr is not actually a block.

@petrochenkov One issue with this is that prefix operators have high precedence, so the suggestion for wrapping the code in a block doesn't capture what it should:

error: expected inline const block
--> $DIR/inline-const-without-block.rs:50:19
|
LL | let y = const 1 + 2 * 3 / 4;
| ^ help: try placing this code inside a block: `{ 1 }`

I'm not sure if there's a good way to resolve this because of the precedence of prefix operators. I haven't made many changes to the parser before, so I would appreciate some extra guidance! Thanks :)

[petrochenkov]

const { 1 } is the behavior that I expected, if const starts working without braces like box, then it's exactly the priority that it will have.
We also don't know what it "should" capture in general case - code like const { f() } + 1 is as reasonable as const { f() + 1 }.

[petrochenkov]

Closing due to inactivity.