Rustc passes syntactically invalid input to attribute macros

[dtolnay]

It used to be the case that attribute macros would only ever be invoked with syntactically valid input. Rustc would snip out or otherwise fix up invalid syntax prior to dispatching the macro invocation. This ensures that rustc's recovery from the invalid syntax, and any diagnostics emitted by rustc to that effect, are consistent with any subsequent diagnostics emitted by the macro, rather than requiring both to perform independent and inconsistent heuristics to recover from the invalid input.

However, it looks like recent rustc versions (1.51+) have begun invoking attribute macros on invalid syntax. Can we find out whether this behavior is intended or a regression from recent Span-related work (#43081 — @Aaron1011)? IMO the newfangled behavior is not necessarily desirable so I have filed this as a regression.

// src/lib.rs

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn repro(_args: TokenStream, input: TokenStream) -> TokenStream {
    eprintln!("{:#?}", input);
    TokenStream::new()
}

// src/main.rs

#[repro::repro]
trait T {
    fn run(&);
}

fn main() {}

# Cargo.toml

[package]
name = "repro"
version = "0.0.0"
edition = "2018"
publish = false

[lib]
proc-macro = true

• 1.31 through 1.39: macro is invoked with trait T {} as input. Invalid syntax has been snipped out.
• 1.40 through 1.50: macro is invoked with trait T { fn run(); } as input, invalid syntax snipped out with finer granularity.
• 1.51+: macro is invoked with trait T { fn run(&); } which is invalid syntax and not necessarily desirable.

[Aaron1011]

This is probably due to parser recovery - in all of the affected versions, the parser was able to parse an Item despite the invalid syntax. In the past, we relied on pretty-printing and preparing in many cases, and pretty-printing a parsed item should always produce valid syntax. The output produced would depend on the precise way in which recovery was performed (e.g do we ignore the parameter, or the entire method?).

Now, we no longer rely on pretty-printing when incoming macros. The original (syntactically invalid) tokenstream is passed to the macro, without any kind of modification.

[Aaron1011]

See #76360 (comment) for a previous discussion of this kind of issue.

[dtolnay]

Thanks for the link! The behavior I would expect is for the attribute macro to be run on the prettyprinted recovered input, iow the invalid node is snipped out or otherwise fixed up prior to the macro expansion. I think this is described as option 1 in your post. I left a comment on the other issue to that effect.

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

[pnkfelix]

Discussed at P-high review

This seems like a real problem. I'm asking @estebank and @compiler-errors to try to dedicate some brain cycles to looking over the suggestions here and trying to drive progress towards a solution.

[pnkfelix]

Discussed at 2023 Q1 P-high review

Arguably this is folded in with #76360, which has already (also) been handed off to WG-diagnostics, specifically @estebank and @compiler-errors , for some dedicated brain cycle time.