Skip to content

Commit 783d4b8

Browse files
committed
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.
1 parent 9bd71af commit 783d4b8

File tree

2 files changed

+44
-30
lines changed

2 files changed

+44
-30
lines changed

compiler/rustc_ast_pretty/src/pprust/state.rs

+42-30
Original file line numberDiff line numberDiff line change
@@ -146,37 +146,49 @@ pub fn print_crate<'a>(
146146
s.s.eof()
147147
}
148148

149-
/// This makes printed token streams look slightly nicer,
150-
/// and also addresses some specific regressions described in #63896 and #73345.
151-
fn space_between(prev: &TokenTree, curr: &TokenTree) -> bool {
152-
if let TokenTree::Token(token, _) = prev {
153-
// No space after these tokens, e.g. `x.y`, `$e`
154-
// (The carets point to `prev`.) ^ ^
155-
if matches!(token.kind, token::Dot | token::Dollar) {
156-
return false;
157-
}
158-
if let token::DocComment(comment_kind, ..) = token.kind {
159-
return comment_kind != CommentKind::Line;
160-
}
161-
}
162-
match curr {
163-
// No space before these tokens, e.g. `foo,`, `println!`, `x.y`
164-
// (The carets point to `curr`.) ^ ^ ^
149+
/// Should two consecutive tokens be printed with a space between them?
150+
///
151+
/// Note: some old proc macros parse pretty-printed output, so changes here can
152+
/// break old code. For example:
153+
/// - #63896: `#[allow(unused,` must be printed rather than `#[allow(unused ,`
154+
/// - #73345: `#[allow(unused)] must be printed rather than `# [allow(unused)]
155+
///
156+
fn space_between(tt1: &TokenTree, tt2: &TokenTree) -> bool {
157+
use token::*;
158+
use Delimiter::*;
159+
use TokenTree::Delimited as Del;
160+
use TokenTree::Token as Tok;
161+
162+
// Each match arm has one or more examples in comments. The default is to
163+
// insert space between adjacent tokens, except for the cases listed in
164+
// this match.
165+
match (tt1, tt2) {
166+
// No space after line doc comments.
167+
(Tok(Token { kind: DocComment(CommentKind::Line, ..), .. }, _), _) => false,
168+
169+
// `.` + ANYTHING: `x.y`, `tup.0`
170+
// `$` + ANYTHING: `$e`
171+
(Tok(Token { kind: Dot | Dollar, .. }, _), _) => false,
172+
173+
// ANYTHING + `,`: `foo,`
174+
// ANYTHING + `.`: `x.y`, `tup.0`
175+
// ANYTHING + `!`: `foo! { ... }`
165176
//
166-
// FIXME: having `Not` here works well for macro invocations like
167-
// `println!()`, but is bad when `!` means "logical not" or "the never
168-
// type", where the lack of space causes ugliness like this:
169-
// `Fn() ->!`, `x =! y`, `if! x { f(); }`.
170-
TokenTree::Token(token, _) => !matches!(token.kind, token::Comma | token::Not | token::Dot),
171-
// No space before parentheses if preceded by these tokens, e.g. `foo(...)`
172-
TokenTree::Delimited(_, Delimiter::Parenthesis, _) => {
173-
!matches!(prev, TokenTree::Token(Token { kind: token::Ident(..), .. }, _))
174-
}
175-
// No space before brackets if preceded by these tokens, e.g. `#[...]`
176-
TokenTree::Delimited(_, Delimiter::Bracket, _) => {
177-
!matches!(prev, TokenTree::Token(Token { kind: token::Pound, .. }, _))
178-
}
179-
TokenTree::Delimited(..) => true,
177+
// FIXME: Incorrect cases:
178+
// - Logical not: `x =! y`, `if! x { f(); }`
179+
// - Never type: `Fn() ->!`
180+
(_, Tok(Token { kind: Comma | Dot | Not, .. }, _)) => false,
181+
182+
// IDENT + `(`: `f(3)`
183+
//
184+
// FIXME: Incorrect cases:
185+
// - Let: `let(a, b) = (1, 2)`
186+
(Tok(Token { kind: Ident(..), .. }, _), Del(_, Parenthesis, _)) => false,
187+
188+
// `#` + `[`: `#[attr]`
189+
(Tok(Token { kind: Pound, .. }, _), Del(_, Bracket, _)) => false,
190+
191+
_ => true,
180192
}
181193
}
182194

tests/ui/macros/stringify.rs

+2
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ fn test_expr() {
115115
"a + b * c - d + -1 * -2 - -3",
116116
"a + b * c - d + - 1 * - 2 - - 3"
117117
);
118+
c2!(expr, [ x = !y ], "x = !y", "x =! y"); // FIXME
118119

119120
// ExprKind::Unary
120121
c2!(expr, [ *expr ], "*expr", "* expr");
@@ -137,6 +138,7 @@ fn test_expr() {
137138

138139
// ExprKind::If
139140
c1!(expr, [ if true {} ], "if true {}");
141+
c2!(expr, [ if !true {} ], "if !true {}", "if! true {}"); // FIXME
140142
c2!(expr,
141143
[ if ::std::blah() { } else { } ],
142144
"if ::std::blah() {} else {}",

0 commit comments

Comments
 (0)