Use token::Lit in ast::ExprKind::Lit.

[nnethercote]

Instead of ast::Lit.

Literal lowering now happens at two different times. Expression literals are lowered when HIR is crated. Attribute literals are lowered during parsing.

r? @petrochenkov

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[nnethercote]

Another alternative to the literal changes in #101562 and #101885.

I don't feel entirely happy with this approach. Various things:

• The output change to src/test/ui/lint/lint-impl-fn.rs is bogus, but I don't understand why it happens.
• rustc_ast_lowering now depends on rustc_parse, for report_lit_error, because lit lowering now happens in both of those crates.
• ast::Lit still has redundancy, which two literal "kinds" within it.
• There are some almost duplicate paths in the parser:
  • parse_opt_token_lit / parse_opt_ast_lit
  • parse_token_lit / parse_ast_lit`

[nnethercote]

Also, it's not yet complete. I haven't updated clippy and rustfmt. I'll do that if we decide that this is the right path forward.

[petrochenkov]

rustc_ast_lowering now depends on rustc_parse, for report_lit_error, because lit lowering now happens in both of those crates.

The literal lowering logic, including error reporting, is independent of the rest of the parser and can be moved to some common dependency of AST->HIR lowering and Attribute -> MetaItem lowering, or passed through a function pointer like nt_to_tokenstream before #97251.

ast::Lit still has redundancy, which two literal "kinds" within it.

I'd expect it to be renamed to something like MetaItemLit, and the token_lit part to be removed (not in this PR).

[petrochenkov]

Change description for the lang team.

This PR changes some errors related to literals from syntactic-but-not-in-macros to semantic.
These are the error names in the compiler:

    InvalidSuffix,
    InvalidIntSuffix,
    InvalidFloatSuffix,
    NonDecimalFloat(u32),
    IntTooLarge,

Examples:

"string"any_suffix
10u123
10.0f123
0b10f32
999340282366920938463463374607431768211455999

All these examples are valid tokens, that can be passed to macros without errors, but previously they were reported as errors when parsing proper Rust code, including code that will be later removed by cfg or attribute macros.

// Before
sink! {
    "string"any_suffix // OK
    10u123 // OK
    10.0f123 // OK
    0b10f32 // OK
    999340282366920938463463374607431768211455999 // OK
}

#[cfg(FALSE)]
fn configured_out() {
    "string"any_suffix // ERROR
    10u123 // ERROR
    10.0f123 // ERROR
    0b10f32 // ERROR
    999340282366920938463463374607431768211455999 // ERROR
}

fn main() {
    "string"any_suffix // ERROR
    10u123 // ERROR
    10.0f123 // ERROR
    0b10f32 // ERROR
    999340282366920938463463374607431768211455999 // ERROR
}

This PR makes them fully semantic errors so they are only reported after all macros are expanded.

// After
sink! {
    "string"any_suffix // OK
    10u123 // OK
    10.0f123 // OK
    0b10f32 // OK
    999340282366920938463463374607431768211455999 // OK
}

#[cfg(FALSE)]
fn configured_out() {
    "string"any_suffix // OK
    10u123 // OK
    10.0f123 // OK
    0b10f32 // OK
    999340282366920938463463374607431768211455999 // OK
}

fn main() {
    "string"any_suffix // ERROR
    10u123 // ERROR
    10.0f123 // ERROR
    0b10f32 // ERROR
    999340282366920938463463374607431768211455999 // ERROR
}

This way

• we make behavior of function-like and attribute-like macros more consistent
• we make other literals more consistent with floating point literals which are already converted from strings to floats lately in the compiler (probably due to expensiveness of the conversion)
• we avoid unnecessary work in the compiler - literals don't need to be checked/converted unless they are really used, and size of expression AST nodes can be shrank.

[nnethercote]

we make other literals more consistent with floating point literals which are already converted from strings to floats lately in the compiler (probably due to expensiveness of the conversion)

Floats are stored as Symbols rather than f64 in ast::LitKind so that ast::LitKind can implement Eq and Hash. I think this is the real reason they are converted later, rather than cost.

[BennoLossin]

0x10.0 // OK

This is currently not accepted by the compiler: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=316edfc43c4ad955ee7b667883a10287

[petrochenkov]

@y86-dev
Ah, you are right, 0x10.0 -> 0b10f32 in all the examples.
UPD: I've updated the examples in #102944 (comment).

[nnethercote]

@scottmcm: The meeting notes indicate some support for this, but also that a firm conclusion wasn't reached. Should we do an FCP?

[scottmcm]

@nnethercote I've been trying to distill my thoughts here into something less fuzzy. I think I see three different situations here.

1. Things that match currently-existing token forms, but are hitting arbitrary length restrictions.

For example, decimal literals that are obviously lexically fine, like

#[cfg(nope)]
pub fn foo() {
    let x = 1111111111222222222233333333334444444444;
}

For that kind, I think we had lang consensus that they definitely shouldn't be a tokenizing nor parsing time error, but rather a semantic error later. So making those work for macros and cfg'd-out code as I understand this PR would do makes great sense to me.

2. Extensions of existing lexical forms to other widths.

I don't think we're likely to add u1024 any time soon, but if we ever did I think it's very obvious that 0_u1024 would be a valid value of the type, as would 0xFFFF_u1024 and the other obvious examples. Similarly for 0.0_f16, or other such things with different bitwidths. (0.0_f11 is massively unlikely, but I still don't think it's something we'd need to block.)

I think lang also felt positive about supporting these, though I don't think we talked about them as much. Personally I'm definitely in favour.

3. Things where the combination has no meaning today.

This is where I'm not so confident about things. When it explicitly rejects hexadecimal and binary floating point, it's not at all obvious to me that 0b0f32 is something we want to support for situations like this. What does that even mean, especially when 0x0f32 does something completely related to what an f32 suffix means near a .? Similarly, it's not obvious to me at all that "hello"match should even be one token. I guess we can't break that in macros until an edition, so it's stuck being passable there, but that doesn't mean it's something that makes sense to me to allow through the parser.

---

Hopefully that's understandable 🤞

How do you feel about those distinctions? Is the line between cases 1&2 and case 3 something that you think is meaningful? How obnoxious would it be to separate those cases in the code? Am I missing something about how Rust is set up lexically/syntactically?

cc @rust-lang/lang for comments in hopes this makes more sense than my rambling in the meeting.

[nnethercote]

I have no strong opinions about the language design issues here. I'm just interested in shrinking the size of ast::Literal, in order to shrink ast::Expr, in order to reduce compile times.

@petrochenkov: what do think about Scott's comments?

[petrochenkov]

@scottmcm

How do you feel about those distinctions? Is the line between cases 1&2 and case 3 something that you think is meaningful?

I wouldn't make any of these distinctions.
For me the important part is that all of them are considered valid tokens (accepted as tt or proc macro input), so it's too late to make the distinctions and they should all be accepted by Rust parser as well.
If you want to make distinctions, then perhaps the moral of this story is that we need to pay more attention when stabilizing something as a valid token.

How obnoxious would it be to separate those cases in the code? Am I missing something about how Rust is set up lexically/syntactically?

• "string"any_suffix - less obnoxious, the suffix check is cheap and AST expressions can still keep token::Lit, although it may be annoying to not report this error twice.
• everything else - more obnoxious, AST either needs to keep lowered literals, or the compiler will have to lower literals twice doing more work instead of less work.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting, and we decided that we're willing to evaluate this via FCP. We're not thrilled that these were handled in the first place, but given that they work in macros, we should let them work in cfg(FALSE). That doesn't stop us from giving them language-level meanings in the parser later.

[joshtriplett]

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nnethercote]

I rebased.

@bors r=petrochenkov

[bors]

📌 Commit 358a603 has been approved by petrochenkov

It is now in the queue for this repository.

[Manishearth]

@bors p=1

[bors]

⌛ Testing commit 358a603 with merge bebd57a...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing bebd57a to master...

[rust-timer]

Finished benchmarking commit (bebd57a): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.7%]	16
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	1
Improvements ✅ (secondary)	-0.5%	[-1.0%, -0.3%]	10
All ❌✅ (primary)	0.3%	[-0.2%, 0.7%]	17

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.5%, 0.5%]	1
Regressions ❌ (secondary)	2.5%	[1.6%, 3.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-3.1%, -3.1%]	1
All ❌✅ (primary)	0.5%	[0.5%, 0.5%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

[nnethercote]

As above:

The wins and losses roughly balance out. Also, this will enable some additional improvements by shrinking the size of ast::Expr.

@rustbot label: +perf-regression-triaged

[rylev]

Note from perf triage: the regressions are largely concentrated in primary benchmarks while the improvements are in secondary. So while the regressions and improvements balance out when treating all benchmarks equally, I do think that primary regressions should be considered to have more weight than secondary improvements.