Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change struct expr pretty printing to match rustfmt style #93556

Merged
merged 5 commits into from
Feb 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions compiler/rustc_ast_pretty/src/pp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub enum Breaks {
Inconsistent,
}

#[derive(Clone, Copy)]
#[derive(Clone, Copy, PartialEq)]
enum IndentStyle {
/// Vertically aligned under whatever column this block begins at.
///
Expand All @@ -164,19 +164,20 @@ enum IndentStyle {
Block { offset: isize },
}

#[derive(Clone, Copy)]
#[derive(Clone, Copy, Default, PartialEq)]
pub struct BreakToken {
offset: isize,
blank_space: isize,
pre_break: Option<char>,
}

#[derive(Clone, Copy)]
#[derive(Clone, Copy, PartialEq)]
pub struct BeginToken {
indent: IndentStyle,
breaks: Breaks,
}

#[derive(Clone)]
#[derive(Clone, PartialEq)]
pub enum Token {
// In practice a string token contains either a `&'static str` or a
// `String`. `Cow` is overkill for this because we never modify the data,
Expand Down Expand Up @@ -313,6 +314,12 @@ impl Printer {
}
}

pub fn offset(&mut self, offset: isize) {
if let Some(BufEntry { token: Token::Break(token), .. }) = &mut self.buf.last_mut() {
token.offset += offset;
}
}

fn check_stream(&mut self) {
while self.right_total - self.left_total > self.space {
if *self.scan_stack.front().unwrap() == self.buf.index_of_first() {
Expand Down Expand Up @@ -391,7 +398,9 @@ impl Printer {
if size > self.space {
self.print_stack.push(PrintFrame::Broken { indent: self.indent, breaks: token.breaks });
self.indent = match token.indent {
IndentStyle::Block { offset } => (self.indent as isize + offset) as usize,
IndentStyle::Block { offset } => {
usize::try_from(self.indent as isize + offset).unwrap()
}
IndentStyle::Visual => (MARGIN - self.space) as usize,
};
} else {
Expand All @@ -415,6 +424,9 @@ impl Printer {
self.pending_indentation += token.blank_space;
self.space -= token.blank_space;
} else {
if let Some(pre_break) = token.pre_break {
self.out.push(pre_break);
}
self.out.push('\n');
let indent = self.indent as isize + token.offset;
self.pending_indentation = indent;
Expand Down
33 changes: 23 additions & 10 deletions compiler/rustc_ast_pretty/src/pp/convenience.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ use std::borrow::Cow;

impl Printer {
/// "raw box"
pub fn rbox(&mut self, indent: usize, breaks: Breaks) {
self.scan_begin(BeginToken {
indent: IndentStyle::Block { offset: indent as isize },
breaks,
})
pub fn rbox(&mut self, indent: isize, breaks: Breaks) {
self.scan_begin(BeginToken { indent: IndentStyle::Block { offset: indent }, breaks })
}

/// Inconsistent breaking box
pub fn ibox(&mut self, indent: usize) {
pub fn ibox(&mut self, indent: isize) {
self.rbox(indent, Breaks::Inconsistent)
}

/// Consistent breaking box
pub fn cbox(&mut self, indent: usize) {
pub fn cbox(&mut self, indent: isize) {
self.rbox(indent, Breaks::Consistent)
}

Expand All @@ -25,7 +22,11 @@ impl Printer {
}

pub fn break_offset(&mut self, n: usize, off: isize) {
self.scan_break(BreakToken { offset: off, blank_space: n as isize })
self.scan_break(BreakToken {
offset: off,
blank_space: n as isize,
..BreakToken::default()
});
}

pub fn end(&mut self) {
Expand Down Expand Up @@ -66,12 +67,24 @@ impl Printer {
}

pub fn hardbreak_tok_offset(off: isize) -> Token {
Token::Break(BreakToken { offset: off, blank_space: SIZE_INFINITY })
Token::Break(BreakToken {
offset: off,
blank_space: SIZE_INFINITY,
..BreakToken::default()
})
}

pub fn trailing_comma(&mut self) {
self.scan_break(BreakToken {
blank_space: 1,
pre_break: Some(','),
..BreakToken::default()
});
}
}

impl Token {
pub fn is_hardbreak_tok(&self) -> bool {
matches!(self, Token::Break(BreakToken { offset: 0, blank_space: SIZE_INFINITY }))
*self == Printer::hardbreak_tok_offset(0)
}
}
5 changes: 4 additions & 1 deletion compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod delimited;
mod expr;
mod item;

Expand All @@ -23,6 +24,8 @@ use rustc_span::{BytePos, FileName, Span};

use std::borrow::Cow;

pub use self::delimited::IterDelimited;

pub enum MacHeader<'a> {
Path(&'a ast::Path),
Keyword(&'static str),
Expand Down Expand Up @@ -92,7 +95,7 @@ pub struct State<'a> {
ann: &'a (dyn PpAnn + 'a),
}

crate const INDENT_UNIT: usize = 4;
crate const INDENT_UNIT: isize = 4;

/// Requires you to pass an input filename and reader so that
/// it can scan the input text for comments to copy forward.
Expand Down
41 changes: 41 additions & 0 deletions compiler/rustc_ast_pretty/src/pprust/state/delimited.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use std::iter::Peekable;
use std::mem;
use std::ops::Deref;

pub struct Delimited<I: Iterator> {
is_first: bool,
iter: Peekable<I>,
}

pub trait IterDelimited: Iterator + Sized {
fn delimited(self) -> Delimited<Self> {
Delimited { is_first: true, iter: self.peekable() }
}
}

impl<I: Iterator> IterDelimited for I {}

pub struct IteratorItem<T> {
value: T,
pub is_first: bool,
pub is_last: bool,
}

impl<I: Iterator> Iterator for Delimited<I> {
type Item = IteratorItem<I::Item>;

fn next(&mut self) -> Option<Self::Item> {
let value = self.iter.next()?;
let is_first = mem::replace(&mut self.is_first, false);
let is_last = self.iter.peek().is_none();
Some(IteratorItem { value, is_first, is_last })
}
}

impl<T> Deref for IteratorItem<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.value
}
}
70 changes: 39 additions & 31 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::pp::Breaks::{Consistent, Inconsistent};
use crate::pprust::state::{AnnNode, PrintState, State, INDENT_UNIT};
use crate::pp::Breaks::Inconsistent;
use crate::pprust::state::{AnnNode, IterDelimited, PrintState, State, INDENT_UNIT};

use rustc_ast::ptr::P;
use rustc_ast::util::parser::{self, AssocOp, Fixity};
Expand Down Expand Up @@ -117,38 +117,46 @@ impl<'a> State<'a> {
} else {
self.print_path(path, true, 0);
}
self.nbsp();
self.word("{");
self.commasep_cmnt(
Consistent,
fields,
|s, field| {
s.print_outer_attributes(&field.attrs);
s.ibox(INDENT_UNIT);
if !field.is_shorthand {
s.print_ident(field.ident);
s.word_space(":");
}
s.print_expr(&field.expr);
s.end();
},
|f| f.span,
);
match rest {
ast::StructRest::Base(_) | ast::StructRest::Rest(_) => {
self.ibox(INDENT_UNIT);
if !fields.is_empty() {
self.word(",");
self.space();
}
self.word("..");
if let ast::StructRest::Base(ref expr) = *rest {
self.print_expr(expr);
}
self.end();
let has_rest = match rest {
ast::StructRest::Base(_) | ast::StructRest::Rest(_) => true,
ast::StructRest::None => false,
};
if fields.is_empty() && !has_rest {
self.word("}");
return;
}
self.cbox(0);
for field in fields.iter().delimited() {
self.maybe_print_comment(field.span.hi());
self.print_outer_attributes(&field.attrs);
if field.is_first {
self.space_if_not_bol();
}
if !field.is_shorthand {
self.print_ident(field.ident);
self.word_nbsp(":");
}
self.print_expr(&field.expr);
if !field.is_last || has_rest {
self.word_space(",");
} else {
self.trailing_comma();
}
ast::StructRest::None if !fields.is_empty() => self.word(","),
_ => {}
}
if has_rest {
if fields.is_empty() {
self.space();
}
self.word("..");
if let ast::StructRest::Base(expr) = rest {
self.print_expr(expr);
}
self.space();
}
self.offset(-INDENT_UNIT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what this negative offset does? I have trouble understanding how this code manages to find the correct indentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The top of https://github.com/rust-lang/rust/blob/1.58.1/compiler/rustc_ast_pretty/src/pp.rs has an overview of the printing algorithm. Very briefly, when you make a sequence of cbox string break string break string break string end calls on the printer, either all of those breaks will turn into linebreaks, or none of them will turn into linebreaks. If they turn into linebreaks, the indentation at each one is the sum of the indentation of all the surrounding boxes.

So if we have:

ibox(4)cbox(0)Struct {breaka: Abreakb: Bbreak}endend

and it does not fit on the line, then you'd expect to get:

Struct {
    a: A
    b: B
    }

That's why every break also holds its own offset which is relative to the sum of the surrounding boxes' indents, so that we can set -4 on the last break and get:

Struct {
    a: A
    b: B
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed explanation. I'm surprised such negative offset facility was not used before, but that's maybe for another refactoring.

self.end();
self.word("}");
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<'a> PrintState<'a> for State<'a> {
}
}

pub const INDENT_UNIT: usize = 4;
pub const INDENT_UNIT: isize = 4;

/// Requires you to pass an input filename and reader so that
/// it can scan the input text for comments to copy forward.
Expand Down
6 changes: 3 additions & 3 deletions src/test/pretty/ast-stmt-expr-attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ fn syntax() {
let _ = #[attr] foo![#! [attr]];
let _ = #[attr] foo! {};
let _ = #[attr] foo! { #! [attr] };
let _ = #[attr] Foo{bar: baz,};
let _ = #[attr] Foo{..foo};
let _ = #[attr] Foo{bar: baz, ..foo};
let _ = #[attr] Foo { bar: baz };
let _ = #[attr] Foo { ..foo };
let _ = #[attr] Foo { bar: baz, ..foo };
let _ = #[attr] (0);

{
Expand Down
5 changes: 3 additions & 2 deletions src/test/pretty/issue-68710-field-attr-proc-mac-lost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ struct C {

#[allow()]
const C: C =
C{
C {
#[cfg(debug_assertions)]
field: 0,

#[cfg(not(debug_assertions))]
field: 1,};
field: 1,
};
12 changes: 6 additions & 6 deletions src/test/pretty/stmt_expr_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ struct Bar(());
fn _7() {

#[rustc_dummy]
Foo{data: (),};
Foo { data: () };

let _ = #[rustc_dummy] Foo{data: (),};
let _ = #[rustc_dummy] Foo { data: () };
}

fn _8() {
Expand Down Expand Up @@ -209,7 +209,7 @@ fn _11() {
let mut x = 0;
let _ = #[rustc_dummy] x = 15;
let _ = #[rustc_dummy] x += 15;
let s = Foo{data: (),};
let s = Foo { data: () };
let _ = #[rustc_dummy] s.data;
let _ = (#[rustc_dummy] s).data;
let t = Bar(());
Expand All @@ -235,9 +235,9 @@ fn _11() {
let _ = #[rustc_dummy] expr_mac!();
let _ = #[rustc_dummy] expr_mac![];
let _ = #[rustc_dummy] expr_mac! {};
let _ = #[rustc_dummy] Foo{data: (),};
let _ = #[rustc_dummy] Foo{..s};
let _ = #[rustc_dummy] Foo{data: (), ..s};
let _ = #[rustc_dummy] Foo { data: () };
let _ = #[rustc_dummy] Foo { ..s };
let _ = #[rustc_dummy] Foo { data: (), ..s };
let _ = #[rustc_dummy] (0);
}

Expand Down
20 changes: 10 additions & 10 deletions src/test/ui/macros/stringify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,17 @@ fn test_expr() {
assert_eq!(stringify_expr!(mac! { ... }), "mac! { ... }");

// ExprKind::Struct
assert_eq!(stringify_expr!(Struct {}), "Struct{}"); // FIXME
assert_eq!(stringify_expr!(Struct {}), "Struct {}");
#[rustfmt::skip] // https://github.com/rust-lang/rustfmt/issues/5151
assert_eq!(stringify_expr!(<Struct as Trait>::Type {}), "<Struct as Trait>::Type{}");
assert_eq!(stringify_expr!(Struct { .. }), "Struct{..}"); // FIXME
assert_eq!(stringify_expr!(Struct { ..base }), "Struct{..base}"); // FIXME
assert_eq!(stringify_expr!(Struct { x }), "Struct{x,}");
assert_eq!(stringify_expr!(Struct { x, .. }), "Struct{x, ..}");
assert_eq!(stringify_expr!(Struct { x, ..base }), "Struct{x, ..base}");
assert_eq!(stringify_expr!(Struct { x: true }), "Struct{x: true,}");
assert_eq!(stringify_expr!(Struct { x: true, .. }), "Struct{x: true, ..}");
assert_eq!(stringify_expr!(Struct { x: true, ..base }), "Struct{x: true, ..base}");
assert_eq!(stringify_expr!(<Struct as Trait>::Type {}), "<Struct as Trait>::Type {}");
assert_eq!(stringify_expr!(Struct { .. }), "Struct { .. }");
assert_eq!(stringify_expr!(Struct { ..base }), "Struct { ..base }");
assert_eq!(stringify_expr!(Struct { x }), "Struct { x }");
assert_eq!(stringify_expr!(Struct { x, .. }), "Struct { x, .. }");
assert_eq!(stringify_expr!(Struct { x, ..base }), "Struct { x, ..base }");
assert_eq!(stringify_expr!(Struct { x: true }), "Struct { x: true }");
assert_eq!(stringify_expr!(Struct { x: true, .. }), "Struct { x: true, .. }");
assert_eq!(stringify_expr!(Struct { x: true, ..base }), "Struct { x: true, ..base }");

// ExprKind::Repeat
assert_eq!(stringify_expr!([(); 0]), "[(); 0]");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[$DIR/dbg-macro-expected-behavior.rs:20] Unit = Unit
[$DIR/dbg-macro-expected-behavior.rs:21] a = Unit
[$DIR/dbg-macro-expected-behavior.rs:27] Point{x: 42, y: 24,} = Point {
[$DIR/dbg-macro-expected-behavior.rs:27] Point { x: 42, y: 24 } = Point {
x: 42,
y: 24,
}
Expand Down