Auto merge of rust-lang#117433 - nnethercote:space_between, r=<try>

Improve `print_tts` by making `space_between` smarter

`space_between` currently handles a few cases that make the output nicer. It also gets some cases wrong. This PR fixes the wrong cases, and adds a bunch of extra cases, resulting in prettier output. E.g. these lines:
```
use smallvec :: SmallVec ;
assert! (mem :: size_of :: < T > () != 0) ;
```
become these lines:
```
use smallvec::SmallVec;
assert!(mem::size_of:: < T >() != 0);
```
This overlaps with rust-lang#114571, but this PR has the crucial characteristic of giving the same results for all token streams, including those generated by proc macros. For that reason I think it's worth having even if/when rust-lang#114571 is merged. It's also nice that this PR's improvements can be obtained by modifying only `space_between`.

r? `@petrochenkov`

Author: bors

compiler/rustc_ast_pretty/src/lib.rs

@@ -5,6 +5,8 @@
 #![deny(rustc::diagnostic_outside_of_impl)]
 #![feature(associated_type_bounds)]
 #![feature(box_patterns)]
+#![feature(if_let_guard)]
+#![feature(let_chains)]
 #![feature(with_negative_coherence)]
 #![recursion_limit = "256"]
 

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -146,37 +146,144 @@ 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`.)          ^           ^     ^
-        //
-        // 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,
+fn is_punct(tt: &TokenTree) -> bool {
+    matches!(tt, TokenTree::Token(tok, _) if tok.is_punct())
+}
+
+/// Should two consecutive token trees be printed with a space between them?
+///
+/// NOTE: should always be false if both token trees are punctuation, so that
+/// any old proc macro that parses pretty-printed code won't glue together
+/// tokens that shouldn't be glued.
+///
+/// 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(prev: Option<&TokenTree>, 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.
+    match (tt1, tt2) {
+        // No space after line doc comments.
+        (Tok(Token { kind: DocComment(CommentKind::Line, ..), .. }, _), _) => false,
+
+        // `.` + NON-PUNCT: `x.y`, `tup.0`
+        // `$` + NON-PUNCT: `$e`
+        (Tok(Token { kind: Dot | Dollar, .. }, _), tt2) if !is_punct(tt2) => false,
+
+        // NON-PUNCT + `,`: `foo,`
+        // NON-PUNCT + `;`: `x = 3;`, `[T; 3]`
+        // NON-PUNCT + `.`: `x.y`, `tup.0`
+        // NON-PUNCT + `:`: `'a: loop { ... }`, `x: u8`, `where T: U`,
+        //     `<Self as T>::x`, `Trait<'a>: Sized`, `X<Y<Z>>: Send`,
+        //     `let (a, b): (u32, u32);`
+        (tt1, Tok(Token { kind: Comma | Semi | Dot | Colon, .. }, _)) if !is_punct(tt1) => false,
+
+        // ANYTHING-BUT-`,`|`:`|`mut`|`<` + `[`: `<expr>[1]`, `vec![]`, `#[attr]`,
+        //     `#![attr]`, but not `data: [T; 0]`, `f(a, [])`, `&mut [T]`,
+        //     `NonNull< [T] >`
+        (Tok(Token { kind: Comma | Colon | Lt, .. }, _), Del(_, Bracket, _)) => true,
+        (Tok(Token { kind: Ident(sym, is_raw), .. }, _), Del(_, Bracket, _))
+            if *sym == kw::Mut && !is_raw =>
+        {
+            true
+        }
+        (Tok(_, _), Del(_, Bracket, _)) => false,
+
+        // IDENT|`fn`|`Self`|`pub` + `(`: `f(3)`, `fn(x: u8)`, `Self()`, `pub(crate)`,
+        //      but `let (a, b) = (1, 2)` needs a space after the `let`
+        (Tok(Token { kind: Ident(sym, is_raw), span }, _), Del(_, Parenthesis, _))
+            if !Ident::new(*sym, *span).is_reserved()
+                || *sym == kw::Fn
+                || *sym == kw::SelfUpper
+                || *sym == kw::Pub
+                || *is_raw =>
+        {
+            false
+        }
+
+        // IDENT|`self`|`Self`|`$crate`|`crate`|`super` + `::`: `x::y`,
+        //     `Self::a`, `$crate::x`, `crate::x`, `super::x`, but
+        //     `if ::a::b() { ... }` needs a space after the `if`.
+        (Tok(Token { kind: Ident(sym, is_raw), span }, _), Tok(Token { kind: ModSep, .. }, _))
+            if !Ident::new(*sym, *span).is_reserved()
+                || sym.is_path_segment_keyword()
+                || *is_raw =>
+        {
+            false
+        }
+
+        // `::` + IDENT: `foo::bar`
+        // `::` + `{`: `use a::{b, c}`
+        (
+            Tok(Token { kind: ModSep, .. }, _),
+            Tok(Token { kind: Ident(..), .. }, _) | Del(_, Brace, _),
+        ) => false,
+
+        // `impl` + `<`: `impl<T> Foo<T> { ... }`
+        // `for` + `<`: `for<'a> fn()`
+        (Tok(Token { kind: Ident(sym, is_raw), .. }, _), Tok(Token { kind: Lt, .. }, _))
+            if (*sym == kw::Impl || *sym == kw::For) && !is_raw =>
+        {
+            false
+        }
+
+        // `fn` + IDENT + `<`: `fn f<T>(t: T) { ... }`
+        (Tok(Token { kind: Ident(..), .. }, _), Tok(Token { kind: Lt, .. }, _))
+            if let Some(prev) = prev
+                && let Tok(Token { kind: Ident(sym, is_raw), .. }, _) = prev
+                && *sym == kw::Fn
+                && !is_raw =>
+            {
+                false
+            }
+
+        // `>` + `(`: `f::<u8>()`
+        // `>>` + `(`: `collect::<Vec<_>>()`
+        (Tok(Token { kind: Gt | BinOp(Shr), .. }, _), Del(_, Parenthesis, _)) => false,
+
+        // IDENT + `!`: `println!()`, but `if !x { ... }` needs a space after the `if`
+        (Tok(Token { kind: Ident(sym, is_raw), span }, _), Tok(Token { kind: Not, .. }, _))
+            if !Ident::new(*sym, *span).is_reserved() || *is_raw =>
+        {
+            false
+        }
+
+        // ANYTHING-BUT-`macro_rules` + `!` + NON-PUNCT-OR-BRACE: `foo!()`, `vec![]`,
+        //     `if !cond { ... }`, but not `macro_rules! m { ... }`
+        (Tok(Token { kind: Not, .. }, _), tt2) if is_punct(tt2) => true,
+        (Tok(Token { kind: Not, .. }, _), Del(_, Brace, _)) => true,
+        (Tok(Token { kind: Not, .. }, _), _) =>
+            if let Some(prev) = prev
+                && let Tok(Token { kind: Ident(sym, is_raw), .. }, _) = prev
+                && *sym == sym::macro_rules
+                && !is_raw
+            {
+                true
+            } else {
+                false
+            }
+
+        // `~` + `const`: `impl ~const Clone`
+        (Tok(Token { kind: Tilde, .. }, _), Tok(Token { kind: Ident(sym, is_raw), .. }, _))
+            if *sym == kw::Const && !is_raw =>
+        {
+            false
+        }
+
+        // `?` + `Sized`: `dyn ?Sized`
+        (Tok(Token { kind: Question, .. }, _), Tok(Token { kind: Ident(sym, is_raw), .. }, _))
+            if *sym == sym::Sized && !is_raw =>
+        {
+            false
+        }
+
+        _ => true,
     }
 }
 
@@ -571,14 +678,19 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
     }
 
     fn print_tts(&mut self, tts: &TokenStream, convert_dollar_crate: bool) {
+        let mut prev = None;
         let mut iter = tts.trees().peekable();
         while let Some(tt) = iter.next() {
             self.print_tt(tt, convert_dollar_crate);
             if let Some(next) = iter.peek() {
-                if space_between(tt, next) {
+                if space_between(prev, tt, next) {
                     self.space();
+                } else {
+                    // There must be a space between two punctuation tokens.
+                    assert!(!is_punct(tt) || !is_punct(next));
                 }
             }
+            prev = Some(tt);
         }
     }
 

compiler/rustc_span/src/symbol.rs

@@ -302,6 +302,7 @@ symbols! {
         Saturating,
         Send,
         SeqCst,
+        Sized,
         SliceIndex,
         SliceIter,
         Some,

tests/pretty/ast-stmt-expr-attr.rs

@@ -113,11 +113,11 @@ fn syntax() {
     let _ = #[attr] continue;
     let _ = #[attr] return;
     let _ = #[attr] foo!();
-    let _ = #[attr] foo!(#! [attr]);
+    let _ = #[attr] foo!(# ![attr]);
     let _ = #[attr] foo![];
-    let _ = #[attr] foo![#! [attr]];
+    let _ = #[attr] foo![# ![attr]];
     let _ = #[attr] foo! {};
-    let _ = #[attr] foo! { #! [attr] };
+    let _ = #[attr] foo! { # ![attr] };
     let _ = #[attr] Foo { bar: baz };
     let _ = #[attr] Foo { ..foo };
     let _ = #[attr] Foo { bar: baz, ..foo };

tests/pretty/cast-lt.pp

@@ -8,6 +8,6 @@
 // pretty-mode:expanded
 // pp-exact:cast-lt.pp
 
-macro_rules! negative { ($e : expr) => { $e < 0 } }
+macro_rules! negative { ($e: expr) => { $e < 0 } }
 
 fn main() { (1 as i32) < 0; }

tests/pretty/delimited-token-groups.rs

@@ -2,23 +2,23 @@
 
 #![feature(rustc_attrs)]
 
-macro_rules! mac { ($($tt : tt) *) => () }
+macro_rules! mac { ($($tt: tt) *) => () }
 
 mac! {
-    struct S { field1 : u8, field2 : u16, } impl Clone for S
+    struct S { field1: u8, field2: u16, } impl Clone for S
     {
         fn clone() -> S
         {
-            panic! () ;
+            panic!();
 
         }
     }
 }
 
 mac! {
     a(aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
-    aaaaaaaa aaaaaaaa) a
-    [aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
+    aaaaaaaa aaaaaaaa)
+    a[aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
     aaaaaaaa aaaaaaaa] a
     {
         aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa

tests/pretty/macro.rs

@@ -2,6 +2,6 @@
 
 #![feature(decl_macro)]
 
-pub(crate) macro mac { ($arg : expr) => { $arg + $arg } }
+pub(crate) macro mac { ($arg: expr) => { $arg + $arg } }
 
 fn main() {}

tests/pretty/macro_rules.rs

@@ -1,19 +1,19 @@
 // pp-exact
 
-macro_rules! brace { () => {} ; }
+macro_rules! brace { () => {}; }
 
-macro_rules! bracket[() => {} ;];
+macro_rules! bracket[() => {};];
 
-macro_rules! paren(() => {} ;);
+macro_rules! paren(() => {};);
 
 macro_rules! matcher_brackets {
-    (paren) => {} ; (bracket) => {} ; (brace) => {} ;
+    (paren) => {}; (bracket) => {}; (brace) => {};
 }
 
 macro_rules! all_fragments {
-    ($b : block, $e : expr, $i : ident, $it : item, $l : lifetime, $lit :
-    literal, $m : meta, $p : pat, $pth : path, $s : stmt, $tt : tt, $ty : ty,
-    $vis : vis) => {} ;
+    ($b: block, $e: expr, $i: ident, $it: item, $l: lifetime, $lit: literal,
+    $m: meta, $p: pat, $pth: path, $s: stmt, $tt: tt, $ty: ty, $vis: vis) =>
+    {};
 }
 
 fn main() {}

tests/pretty/offset_of.rs

@@ -1,4 +1,4 @@
 // pp-exact
 #![feature(offset_of)]
 
-fn main() { std::mem::offset_of!(std :: ops :: Range < usize >, end); }
+fn main() { std::mem::offset_of!(std::ops::Range < usize > , end); }

tests/pretty/stmt_expr_attributes.rs

@@ -113,7 +113,7 @@ fn _8() {
 }
 
 fn _9() {
-    macro_rules! stmt_mac { () => { let _ = () ; } }
+    macro_rules! stmt_mac { () => { let _ = (); } }
 
     #[rustc_dummy]
     stmt_mac!();

tests/run-make/rustc-macro-dep-files/foo.rs

@@ -7,6 +7,6 @@ use proc_macro::TokenStream;
 #[proc_macro_derive(A)]
 pub fn derive(input: TokenStream) -> TokenStream {
     let input = input.to_string();
-    assert!(input.contains("struct A ;"));
+    assert!(input.contains("struct A;"));
     "struct B;".parse().unwrap()
 }

tests/ui/async-await/issues/issue-60674.stdout

@@ -1,3 +1,3 @@
-async fn f(mut x : u8) {}
-async fn g((mut x, y, mut z) : (u8, u8, u8)) {}
-async fn g(mut x : u8, (a, mut b, c) : (u8, u8, u8), y : u8) {}
+async fn f(mut x: u8) {}
+async fn g((mut x, y, mut z): (u8, u8, u8)) {}
+async fn g(mut x: u8, (a, mut b, c): (u8, u8, u8), y: u8) {}

tests/ui/hygiene/unpretty-debug.stdout

@@ -8,7 +8,7 @@
 #![feature /* 0#0 */(no_core)]
 #![no_core /* 0#0 */]
 
-macro_rules! foo /* 0#0 */ { ($x : ident) => { y + $x } }
+macro_rules! foo /* 0#0 */ { ($x: ident) => { y + $x } }
 
 fn bar /* 0#0 */() {
     let x /* 0#0 */ = 1;

tests/ui/infinite/issue-41731-infinite-macro-print.stderr

@@ -14,13 +14,13 @@ LL |     stack!("overflow");
    |     ^^^^^^^^^^^^^^^^^^
    |
    = note: expanding `stack! { "overflow" }`
-   = note: to `print! (stack! ("overflow")) ;`
-   = note: expanding `print! { stack! ("overflow") }`
-   = note: to `{ $crate :: io :: _print($crate :: format_args! (stack! ("overflow"))) ; }`
+   = note: to `print!(stack!("overflow"));`
+   = note: expanding `print! { stack!("overflow") }`
+   = note: to `{ $crate::io::_print($crate::format_args!(stack!("overflow"))); }`
    = note: expanding `stack! { "overflow" }`
-   = note: to `print! (stack! ("overflow")) ;`
-   = note: expanding `print! { stack! ("overflow") }`
-   = note: to `{ $crate :: io :: _print($crate :: format_args! (stack! ("overflow"))) ; }`
+   = note: to `print!(stack!("overflow"));`
+   = note: expanding `print! { stack!("overflow") }`
+   = note: to `{ $crate::io::_print($crate::format_args!(stack!("overflow"))); }`
 
 error: format argument must be a string literal
   --> $DIR/issue-41731-infinite-macro-print.rs:14:5

tests/ui/infinite/issue-41731-infinite-macro-println.stderr

@@ -14,13 +14,13 @@ LL |     stack!("overflow");
    |     ^^^^^^^^^^^^^^^^^^
    |
    = note: expanding `stack! { "overflow" }`
-   = note: to `println! (stack! ("overflow")) ;`
-   = note: expanding `println! { stack! ("overflow") }`
-   = note: to `{ $crate :: io :: _print($crate :: format_args_nl! (stack! ("overflow"))) ; }`
+   = note: to `println!(stack!("overflow"));`
+   = note: expanding `println! { stack!("overflow") }`
+   = note: to `{ $crate::io::_print($crate::format_args_nl!(stack!("overflow"))); }`
    = note: expanding `stack! { "overflow" }`
-   = note: to `println! (stack! ("overflow")) ;`
-   = note: expanding `println! { stack! ("overflow") }`
-   = note: to `{ $crate :: io :: _print($crate :: format_args_nl! (stack! ("overflow"))) ; }`
+   = note: to `println!(stack!("overflow"));`
+   = note: expanding `println! { stack!("overflow") }`
+   = note: to `{ $crate::io::_print($crate::format_args_nl!(stack!("overflow"))); }`
 
 error: format argument must be a string literal
   --> $DIR/issue-41731-infinite-macro-println.rs:14:5