Provide suggestions for const arguments missing braces

[estebank]

• Add support for unambiguous expressions in const arguments: foo::<100 - BAR>();
• Account for let expressions in const arguments
• Add partial recovery for some expressions that are parsed as trait object types and suggest to surround with braces: foo::<BAR + BAR>() → foo::<{ BAR + BAR }>()
• Add partial recovery for some expressions that are require braces because they are ambiguous and suggest to surround with braces:
  • foo::<BAR + 3>() → foo::<{ BAR + 3 }>()
  • foo::<BAR - 3>() → foo::<{ BAR - 3 }>()
  • foo::<bar::<i32>() + BAR>() → foo::<{ bar::<i32>() + BAR }>()
  • foo::<BAR - bar::<i32>()>(); → foo::<{ BAR - bar::<i32>() }>();

r? @petrochenkov

[estebank]

CC #64700 & rust-lang/rfcs#1931 (comment)

[lcnr]

This might be because I am used to the current state, so take this with a grain of salt.

I don't feel comfortable with allowing A - B while disallowing A + B.
It feels somewhat inconsistent and would disallow future changes to type arguments without an explicit RFC if we stabilize this.

While this is inconsequential rn, things are more difficult to change once they are added IMO.

[estebank]

@lcnr neither work, they both provide a suggestion. 3 + A will work now, but A + 3 won't. I can see why it could be annoying but this is the behavior originally outlined in the RFC.
I could potentially make the A + B work in the same way that the case of a single ident works (it's parsed as a type with a single path but then during resolution we check to what namespace it belongs to), but that would complicate the grammar further.

[lcnr]

Oh, seems like I misread the test cases for A - B 😅

I can see why it could be annoying but this is the behavior originally outlined in the RFC.

🤔 Afaik the only const generic RFC which was merged is https://github.com/rust-lang/rfcs/blob/master/text/2000-const-generics.md

This seems to forbid integer operations https://github.com/rust-lang/rfcs/blame/master/text/2000-const-generics.md#L114-L118

Any const expression of the type ascribed to a const parameter can be applied
as that parameter. When applying an expression as const parameter (except for
arrays), which is not an identity expression, the expression must be contained
within a block. This syntactic restriction is necessary to avoid requiring
infinite lookahead when parsing an expression inside of a type.

[estebank]

Integers (and other literals like true and "hi") do not require any lookahead because they can never be part of a type so they would always be starting an expression. It follows the strategy outlined in #64700 (comment), which is a summary of the discussion at rust-lang/rfcs#1931 (comment).

I was purposely conservative on what new syntax is added, but the recovery code gives you an idea of what would need to be done if we wanted to be fancy and support things like A + B or const_fn(). In all of those cases some lookahead would be needed or logic to recover from what has already been parsed and recompose the AST (no lookahead, no snapshot needed, but tricky extra logic).

[varkor]

It's really nice to be able to provide nice diagnostics here, but we shouldn't be increasing the supported expressions. The only expressions that are permitted as const generic arguments without curly brackets are literals and identifiers/paths.

It follows the strategy outlined in #64700 (comment), which is a summary of the discussion at rust-lang/rfcs#1931 (comment).

This isn't the merged const generics RFC, though. The merged RFC forbids complex expressions. Even though we can accept some expressions unambiguously, we certainly can't do so consistently (that is, there are some ambiguous expressions). This will be surprising and frustrating for users, who will have little idea what is and is not supported without brackets.

That said, I'm excited to see some big improvements to the diagnostics for const generics! I'll take a closer look soon.

[estebank]

@varkor last commit now forbids the parseable expressions and provides a structured suggestion.

[petrochenkov]

This is a tricky area where it's easy to make language-affecting changes, so it'll take some time and effort to review.
I hope to do it in a few days after cleaning up the rest of the review query.
Sigh, I wish Centril was here.

[petrochenkov]

@estebank
Could you move the new tests and lowering changes (870a7de) into a separate PR?
(An also add diagnostics.rs to rustc_ast_lowering and move the diagnostic code there.)

[estebank]

@petrochenkov let me know which of the two PRs (this and #72273) you think has a better change of being accepted first, so that I can rebase one on top of the other (they are right now "sister" PRs with no common parent).

[petrochenkov]

@estebank
Let's start with #72273.

[bors]

☔ The latest upstream changes (presumably #72276) made this pull request unmergeable. Please resolve the merge conflicts.

[petrochenkov]

This can also be unblocked by removing the #72273 part from this PR.

[petrochenkov]

Closing due to inactivity.