From 783d4b8b2629c31ff8359ce744060afdcb0a5bea Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 31 Oct 2023 11:25:13 +1100 Subject: [PATCH 1/2] Clarify `space_between`. To avoid `!matches!(...)`, which is hard to think about. Instead every case now uses direct pattern matching and returns true or false. Also add a couple of cases to the `stringify.rs` test that currently print badly. --- compiler/rustc_ast_pretty/src/pprust/state.rs | 72 +++++++++++-------- tests/ui/macros/stringify.rs | 2 + 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index e71f421659e2e..48421ff71409c 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -146,37 +146,49 @@ pub fn print_crate<'a>( s.s.eof() } -/// This makes printed token streams look slightly nicer, -/// and also addresses some specific regressions described in #63896 and #73345. -fn space_between(prev: &TokenTree, curr: &TokenTree) -> bool { - if let TokenTree::Token(token, _) = prev { - // No space after these tokens, e.g. `x.y`, `$e` - // (The carets point to `prev`.) ^ ^ - if matches!(token.kind, token::Dot | token::Dollar) { - return false; - } - if let token::DocComment(comment_kind, ..) = token.kind { - return comment_kind != CommentKind::Line; - } - } - match curr { - // No space before these tokens, e.g. `foo,`, `println!`, `x.y` - // (The carets point to `curr`.) ^ ^ ^ +/// Should two consecutive tokens be printed with a space between them? +/// +/// Note: some old proc macros parse pretty-printed output, so changes here can +/// break old code. For example: +/// - #63896: `#[allow(unused,` must be printed rather than `#[allow(unused ,` +/// - #73345: `#[allow(unused)] must be printed rather than `# [allow(unused)] +/// +fn space_between(tt1: &TokenTree, tt2: &TokenTree) -> bool { + use token::*; + use Delimiter::*; + use TokenTree::Delimited as Del; + use TokenTree::Token as Tok; + + // Each match arm has one or more examples in comments. The default is to + // insert space between adjacent tokens, except for the cases listed in + // this match. + match (tt1, tt2) { + // No space after line doc comments. + (Tok(Token { kind: DocComment(CommentKind::Line, ..), .. }, _), _) => false, + + // `.` + ANYTHING: `x.y`, `tup.0` + // `$` + ANYTHING: `$e` + (Tok(Token { kind: Dot | Dollar, .. }, _), _) => false, + + // ANYTHING + `,`: `foo,` + // ANYTHING + `.`: `x.y`, `tup.0` + // ANYTHING + `!`: `foo! { ... }` // - // FIXME: having `Not` here works well for macro invocations like - // `println!()`, but is bad when `!` means "logical not" or "the never - // type", where the lack of space causes ugliness like this: - // `Fn() ->!`, `x =! y`, `if! x { f(); }`. - TokenTree::Token(token, _) => !matches!(token.kind, token::Comma | token::Not | token::Dot), - // No space before parentheses if preceded by these tokens, e.g. `foo(...)` - TokenTree::Delimited(_, Delimiter::Parenthesis, _) => { - !matches!(prev, TokenTree::Token(Token { kind: token::Ident(..), .. }, _)) - } - // No space before brackets if preceded by these tokens, e.g. `#[...]` - TokenTree::Delimited(_, Delimiter::Bracket, _) => { - !matches!(prev, TokenTree::Token(Token { kind: token::Pound, .. }, _)) - } - TokenTree::Delimited(..) => true, + // FIXME: Incorrect cases: + // - Logical not: `x =! y`, `if! x { f(); }` + // - Never type: `Fn() ->!` + (_, Tok(Token { kind: Comma | Dot | Not, .. }, _)) => false, + + // IDENT + `(`: `f(3)` + // + // FIXME: Incorrect cases: + // - Let: `let(a, b) = (1, 2)` + (Tok(Token { kind: Ident(..), .. }, _), Del(_, Parenthesis, _)) => false, + + // `#` + `[`: `#[attr]` + (Tok(Token { kind: Pound, .. }, _), Del(_, Bracket, _)) => false, + + _ => true, } } diff --git a/tests/ui/macros/stringify.rs b/tests/ui/macros/stringify.rs index f1bbd4a621095..31e0d30c78b8f 100644 --- a/tests/ui/macros/stringify.rs +++ b/tests/ui/macros/stringify.rs @@ -115,6 +115,7 @@ fn test_expr() { "a + b * c - d + -1 * -2 - -3", "a + b * c - d + - 1 * - 2 - - 3" ); + c2!(expr, [ x = !y ], "x = !y", "x =! y"); // FIXME // ExprKind::Unary c2!(expr, [ *expr ], "*expr", "* expr"); @@ -137,6 +138,7 @@ fn test_expr() { // ExprKind::If c1!(expr, [ if true {} ], "if true {}"); + c2!(expr, [ if !true {} ], "if !true {}", "if! true {}"); // FIXME c2!(expr, [ if ::std::blah() { } else { } ], "if ::std::blah() {} else {}", From 438b9a6e82cd74102bd41636dc48b6b846101a59 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 1 Nov 2023 08:35:13 +1100 Subject: [PATCH 2/2] More tests for token stream pretty-printing with adjacent punctuation. We currently do the wrong thing on a lot of these. The next commit will fix things. --- tests/ui/macros/stringify.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/ui/macros/stringify.rs b/tests/ui/macros/stringify.rs index 31e0d30c78b8f..70ca00285c49d 100644 --- a/tests/ui/macros/stringify.rs +++ b/tests/ui/macros/stringify.rs @@ -801,3 +801,32 @@ fn test_vis() { assert_eq!(inherited_vis!(struct), ""); assert_eq!(stringify!(), ""); } + +macro_rules! p { + ([$($tt:tt)*], $s:literal) => { + assert_eq!(stringify!($($tt)*), $s); + }; +} + +#[test] +fn test_punct() { + // For all these cases, we must preserve spaces between the tokens. + // Otherwise, any old proc macro that parses pretty-printed code might glue + // together tokens that shouldn't be glued. + p!([ = = < < <= <= == == != != >= >= > > ], "= = < < <= <= == == != != >= >= > >"); + p!([ && && & & || || | | ! ! ], "&& && & & || || | |!!"); // FIXME + p!([ ~ ~ @ @ # # ], "~ ~ @ @ # #"); + p!([ . . .. .. ... ... ..= ..=], ".... .. ... ... ..= ..="); // FIXME + p!([ , , ; ; : : :: :: ], ",, ; ; : : :: ::"); // FIXME + p!([ -> -> <- <- => =>], "-> -> <- <- => =>"); + p!([ $ $ ? ? ' ' ], "$$? ? ' '"); // FIXME + p!([ + + += += - - -= -= * * *= *= / / /= /= ], "+ + += += - - -= -= * * *= *= / / /= /="); + p!([ % % %= %= ^ ^ ^= ^= << << <<= <<= >> >> >>= >>= ], + "% % %= %= ^ ^ ^= ^= << << <<= <<= >> >> >>= >>="); + + // For these one we must insert spaces between adjacent tokens, again due + // to proc macros. + p!([ +! ?= |> >>@ --> <-- $$ =====> ], "+! ? = | > >> @ - -> <- - $$== == =>"); // FIXME + p!([ ,; ;, ** @@ $+$ >< <> ?? +== ], ", ; ;, * * @ @ $+ $> < < > ? ? += ="); // FIXME + p!([ :#!@|$=&*,+;*~? ], ": #! @ | $= & *, + ; * ~ ?"); // FIXME +}