parser: Collect tokens for values in key-value attributes

[petrochenkov]

Fixes #81208 which happens when we parse the attribute value for the second time with an overridden span to synthesize tokens.

It also may be faster to collect tokens instead of re-synthesizing them.

[rust-highfive]

r? @davidtwco

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

[petrochenkov]

@bors try @rust-timer queue
r? @Aaron1011

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit bd07165 with merge 2d1533fed9e0f5f4c94ef0bf054bd1fb1bd8dbfc...

[bors]

☀️ Try build successful - checks-actions
Build commit: 2d1533fed9e0f5f4c94ef0bf054bd1fb1bd8dbfc (2d1533fed9e0f5f4c94ef0bf054bd1fb1bd8dbfc)

[rust-timer]

Queued 2d1533fed9e0f5f4c94ef0bf054bd1fb1bd8dbfc with parent 72c7b70, future comparison URL.

@rustbot label: +S-waiting-on-perf

[Aaron1011]

Can you also update rustc_ast_lowering to stop passing CanSynthesizeMissingTokens::Yes?

[petrochenkov]

@Aaron1011
I tried it, but it causes panics because tokens still need to be synthesized when the value is a result of macro expansion, e.g. #[attr = concat!("a", "b")].

[rust-timer]

Finished benchmarking try commit (2d1533fed9e0f5f4c94ef0bf054bd1fb1bd8dbfc): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

[Aaron1011]

@bors r+

[bors]

📌 Commit bd07165 has been approved by Aaron1011

[bors]

⌛ Testing commit bd07165 with merge 1d0d76f...

[bors]

☀️ Test successful - checks-actions
Approved by: Aaron1011
Pushing 1d0d76f to master...

[ehuss]

@petrochenkov or @Aaron1011, this does not apply cleanly to the beta branch. Do you have some guidance on making it work? The issue is that collect_tokens on beta returns a tuple (expr, tokens), whereas on master it just returns an expr. Just ignoring the tokens didn't seem to work (it ICE'd).

This is somewhat time sensitive since beta will be rolling out soon.

[Aaron1011]

@ehuss: On beta, you should do expr.tokens = Some(tokens).

[ehuss]

Hmm, the following change (on top of this PR):

diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs
index 760e141b1be..501b6d61e61 100644
--- a/compiler/rustc_parse/src/parser/mod.rs
+++ b/compiler/rustc_parse/src/parser/mod.rs
@@ -974,7 +974,8 @@ fn parse_mac_args_common(&mut self, delimited_only: bool) -> PResult<'a, MacArgs
                     }

                     // Collect tokens because they are used during lowering to HIR.
-                    let expr = self.collect_tokens(|this| this.parse_expr())?;
+                    let (mut expr, tokens) = self.collect_tokens(|this| this.parse_expr())?;
+                    expr.tokens = tokens;
                     let span = expr.span;

                     match &expr.kind {

results in the error:

error: byte constant must be ASCII. Use a \xHH escape for a non-ASCII byte
  --> /home/eric/Proj/rust-beta/src/test/ui/attributes/key-value-non-ascii.rs:3:19
   |
LL | #[rustc_dummy = b"ffi.rs"] //~ ERROR byte constant must be ASCII
   |                   ^

thread 'rustc' panicked at 'assertion failed: bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32', compiler/rustc_span/src/lib.rs:1489:17

Did I misinterpret what you meant?

[Aaron1011]

That change looks correct - I'm not sure why the ICE is still happening.
cc @estebank @petrochenkov

[petrochenkov]

The change looks correct to me too (except that we do if expr.tokens.is_none() { expr.tokens = tokens } instead of assigning tokens unconditionally).
I can check what happens, but not earlier than Feb 7.

[petrochenkov]

@Aaron1011 Looks like on beta the pretty-print-reparse hack is not removed yet, so b"ffi.rs" is lexed for the second time (and produces an ICE) unconditionally.

@ehuss I suggest to backport this fragment petrochenkov@e4e460b of #81307 instead, it fixes the underlying problem with span arithmetic.

[pietroalbini]

@petrochenkov cherry-picked that commit into the stable PR!