From c9701d8e189ba97471062147e045e64bba783eb2 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 01:08:02 +0900 Subject: [PATCH 1/9] Update tests --- tests/source/macro_rules.rs | 16 ++++++++++------ tests/target/macro_rules.rs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index ea3cefa8738..17bb3067869 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -1,5 +1,14 @@ // rustfmt-error_on_line_overflow: false +macro_rules! m { + () => (); + ( $ x : ident ) => (); + ( $ m1 : ident , $ m2 : ident , $ x : ident ) => (); + ( $($beginning:ident),*;$middle:ident;$($end:ident),* ) => (); + ( $($beginning: ident),*; $middle: ident; $($end: ident),*; $($beginning: ident),*; $middle: ident; $($end: ident),* ) => {}; + ( $ name : ident ( $ ( $ dol : tt $ var : ident ) * ) $ ( $ body : tt ) * ) => (); +} + macro_rules! m { // a ($expr :expr, $( $func : ident ) * ) => { @@ -88,12 +97,7 @@ macro_rules! m { // #2439 macro_rules! m { - ( - $line0_xxxxxxxxxxxxxxxxx: expr, - $line1_xxxxxxxxxxxxxxxxx: expr, - $line2_xxxxxxxxxxxxxxxxx: expr, - $line3_xxxxxxxxxxxxxxxxx: expr, - ) => {}; + ($line0_xxxxxxxxxxxxxxxxx: expr, $line1_xxxxxxxxxxxxxxxxx: expr, $line2_xxxxxxxxxxxxxxxxx: expr, $line3_xxxxxxxxxxxxxxxxx: expr,) => {}; } // #2466 diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index bc9c6aa4022..5bedc2a8258 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -1,5 +1,21 @@ // rustfmt-error_on_line_overflow: false +macro_rules! m { + () => {}; + ($x: ident) => {}; + ($m1: ident, $m2: ident, $x: ident) => {}; + ($($beginning: ident),*; $middle: ident; $($end: ident),*) => {}; + ( + $($beginning: ident),*; + $middle: ident; + $($end: ident),*; + $($beginning: ident),*; + $middle: ident; + $($end: ident),* + ) => {}; + ($name: ident($($dol: tt $var: ident)*) $($body: tt)*) => {}; +} + macro_rules! m { // a ($expr: expr, $($func: ident)*) => {{ From 84ea306d32f0b9950346108c51c10d2e763530ae Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 01:08:10 +0900 Subject: [PATCH 2/9] Remove unit tests --- src/macros.rs | 47 ----------------------------------------------- 1 file changed, 47 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 030988e5195..1b26c61a820 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -957,50 +957,3 @@ fn format_lazy_static(context: &RewriteContext, shape: Shape, ts: &TokenStream) Some(result) } - -#[cfg(test)] -mod test { - use super::*; - use syntax; - use syntax::parse::{parse_stream_from_source_str, ParseSess}; - use syntax::codemap::{FileName, FilePathMapping}; - - fn format_macro_args_str(s: &str) -> String { - let mut result = String::new(); - syntax::with_globals(|| { - let input = parse_stream_from_source_str( - FileName::Custom("stdin".to_owned()), - s.to_owned(), - &ParseSess::new(FilePathMapping::empty()), - None, - ); - let shape = Shape { - width: 100, - indent: Indent::empty(), - offset: 0, - }; - result = format_macro_args(input.into(), shape).unwrap(); - }); - result - } - - #[test] - fn test_format_macro_args() { - assert_eq!(format_macro_args_str(""), "".to_owned()); - assert_eq!(format_macro_args_str("$ x : ident"), "$x: ident".to_owned()); - assert_eq!( - format_macro_args_str("$ m1 : ident , $ m2 : ident , $ x : ident"), - "$m1: ident, $m2: ident, $x: ident".to_owned() - ); - assert_eq!( - format_macro_args_str("$($beginning:ident),*;$middle:ident;$($end:ident),*"), - "$($beginning: ident),*; $middle: ident; $($end: ident),*".to_owned() - ); - assert_eq!( - format_macro_args_str( - "$ name : ident ( $ ( $ dol : tt $ var : ident ) * ) $ ( $ body : tt ) *" - ), - "$name: ident($($dol: tt $var: ident)*) $($body: tt)*".to_owned() - ); - } -} From ec71459c44045c867bad055d3c954d46fd587a03 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 01:08:18 +0900 Subject: [PATCH 3/9] Format macro arguments with vertical layout --- src/macros.rs | 443 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 384 insertions(+), 59 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 1b26c61a820..d6420af764b 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -463,73 +463,401 @@ fn replace_names(input: &str) -> Option<(String, HashMap)> { Some((result, substs)) } -// This is a bit sketchy. The token rules probably need tweaking, but it works -// for some common cases. I hope the basic logic is sufficient. Note that the -// meaning of some tokens is a bit different here from usual Rust, e.g., `*` -// and `(`/`)` have special meaning. -// -// We always try and format on one line. -// FIXME: Use multi-line when every thing does not fit on one line. -fn format_macro_args(toks: ThinTokenStream, shape: Shape) -> Option { - let mut result = String::with_capacity(128); - let mut insert_space = SpaceState::Never; +#[derive(Debug, Clone)] +enum MacroArgKind { + MetaVariable(ast::Ident, String), + Repeat( + DelimToken, + Vec, + Option>, + Token, + ), + Delimited(DelimToken, Vec), + Separator(String, String), + Other(String, String), +} - for tok in (toks.into(): TokenStream).trees() { - match tok { - TokenTree::Token(_, t) => { - if !result.is_empty() && force_space_before(&t) { - insert_space = SpaceState::Always; - } - if force_no_space_before(&t) { - insert_space = SpaceState::Never; - } - match (insert_space, ident_like(&t)) { - (SpaceState::Always, _) - | (SpaceState::Punctuation, false) - | (SpaceState::Ident, true) => { - result.push(' '); +fn delim_token_to_str( + context: &RewriteContext, + delim_token: &DelimToken, + shape: Shape, + use_multiple_lines: bool, +) -> (String, String) { + let (lhs, rhs) = match *delim_token { + DelimToken::Paren => ("(", ")"), + DelimToken::Bracket => ("[", "]"), + DelimToken::Brace => ("{", "}"), + DelimToken::NoDelim => ("", ""), + }; + if use_multiple_lines { + let indent_str = shape.indent.to_string_with_newline(context.config); + let nested_indent_str = shape + .indent + .block_indent(context.config) + .to_string_with_newline(context.config); + ( + format!("{}{}", lhs, nested_indent_str), + format!("{}{}", indent_str, rhs), + ) + } else { + (lhs.to_owned(), rhs.to_owned()) + } +} + +impl MacroArgKind { + fn starts_with_dollar(&self) -> bool { + match *self { + MacroArgKind::Repeat(..) | MacroArgKind::MetaVariable(..) => true, + _ => false, + } + } + + fn ends_with_space(&self) -> bool { + match *self { + MacroArgKind::Separator(..) => true, + _ => false, + } + } + + fn has_prefix_space(&self) -> bool { + match *self { + MacroArgKind::Separator(_, ref prefix) | MacroArgKind::Other(_, ref prefix) => { + prefix.starts_with(" ") + } + _ => false, + } + } + + fn rewrite( + &self, + context: &RewriteContext, + shape: Shape, + use_multiple_lines: bool, + ) -> Option { + let rewrite_delimited_inner = |delim_tok, args| -> Option<(String, String, String)> { + let (lhs, rhs) = delim_token_to_str(context, delim_tok, shape, false); + let inner = wrap_macro_args(context, args, shape)?; + if lhs.len() + inner.len() + rhs.len() <= shape.width { + return Some((lhs, inner, rhs)); + } + + let (lhs, rhs) = delim_token_to_str(context, delim_tok, shape, true); + let nested_shape = shape + .block_indent(context.config.tab_spaces()) + .with_max_width(context.config); + let inner = wrap_macro_args(context, args, nested_shape)?; + Some((lhs, inner, rhs)) + }; + + match *self { + MacroArgKind::MetaVariable(ty, ref name) => { + Some(format!("${}: {}", name, ty.name.as_str())) + } + MacroArgKind::Repeat(ref delim_tok, ref args, ref another, ref tok) => { + let (lhs, inner, rhs) = rewrite_delimited_inner(delim_tok, args)?; + let another = another + .as_ref() + .and_then(|a| a.rewrite(context, shape, use_multiple_lines)) + .unwrap_or("".to_owned()); + let repeat_tok = pprust::token_to_string(tok); + + Some(format!("${}{}{}{}{}", lhs, inner, rhs, another, repeat_tok)) + } + MacroArgKind::Delimited(ref delim_tok, ref args) => { + rewrite_delimited_inner(delim_tok, args) + .map(|(lhs, inner, rhs)| format!("{}{}{}", lhs, inner, rhs)) + } + MacroArgKind::Separator(ref sep, ref prefix) => Some(format!("{}{} ", prefix, sep)), + MacroArgKind::Other(ref inner, ref prefix) => Some(format!("{}{}", prefix, inner)), + } + } +} + +#[derive(Debug, Clone)] +struct ParsedMacroArg { + kind: MacroArgKind, + span: Span, +} + +impl ParsedMacroArg { + pub fn rewrite( + &self, + context: &RewriteContext, + shape: Shape, + use_multiple_lines: bool, + ) -> Option { + self.kind.rewrite(context, shape, use_multiple_lines) + } +} + +struct MacroArgParser { + lo: BytePos, + hi: BytePos, + buf: String, + is_arg: bool, + last_tok: Token, + start_tok: Token, + result: Vec, +} + +fn last_tok(tt: &TokenTree) -> Token { + match *tt { + TokenTree::Token(_, ref t) => t.clone(), + TokenTree::Delimited(_, ref d) => d.close_token(), + } +} + +impl MacroArgParser { + pub fn new() -> MacroArgParser { + MacroArgParser { + lo: BytePos(0), + hi: BytePos(0), + buf: String::new(), + is_arg: false, + last_tok: Token::Eof, + start_tok: Token::Eof, + result: vec![], + } + } + + fn set_last_tok(&mut self, tok: &TokenTree) { + self.hi = tok.span().hi(); + self.last_tok = last_tok(tok); + } + + fn add_separator(&mut self) { + let prefix = if self.need_space_prefix() { + " ".to_owned() + } else { + "".to_owned() + }; + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Separator(self.buf.clone(), prefix), + span: mk_sp(self.lo, self.hi), + }); + self.buf.clear(); + } + + fn add_other(&mut self) { + let prefix = if self.need_space_prefix() { + " ".to_owned() + } else { + "".to_owned() + }; + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Other(self.buf.clone(), prefix), + span: mk_sp(self.lo, self.hi), + }); + self.buf.clear(); + } + + fn add_meta_variable(&mut self, iter: &mut Cursor) { + match iter.next() { + Some(TokenTree::Token(sp, Token::Ident(ref ident))) => { + self.result.push(ParsedMacroArg { + kind: MacroArgKind::MetaVariable(ident.clone(), self.buf.clone()), + span: mk_sp(self.lo, sp.hi()), + }); + + self.buf.clear(); + self.is_arg = false; + } + _ => unreachable!(), + } + } + + fn update_buffer(&mut self, lo: BytePos, t: &Token) { + if self.buf.is_empty() { + self.lo = lo; + self.start_tok = t.clone(); + } else if force_space_before(t) { + self.buf.push(' '); + } + + self.buf.push_str(&pprust::token_to_string(t)); + } + + fn need_space_prefix(&self) -> bool { + if self.result.is_empty() { + return false; + } + + let last_arg = self.result.last().unwrap(); + if let MacroArgKind::MetaVariable(..) = last_arg.kind { + if ident_like(&self.start_tok) { + return true; + } + } + + if force_space_before(&self.start_tok) { + return true; + } + + false + } + + /// Returns a collection of parsed macro def's arguments. + pub fn parse(mut self, tokens: ThinTokenStream) -> Vec { + let mut iter = (tokens.into(): TokenStream).trees(); + + while let Some(ref tok) = iter.next() { + match tok { + TokenTree::Token(sp, Token::Dollar) => { + // We always want to add a separator before meta variables. + if !self.buf.is_empty() { + self.add_separator(); } - _ => {} + + // Start keeping the name of this metavariable in the buffer. + self.is_arg = true; + self.lo = sp.lo(); + self.start_tok = Token::Dollar; } - result.push_str(&pprust::token_to_string(&t)); - insert_space = next_space(&t); - } - TokenTree::Delimited(_, d) => { - if let SpaceState::Always = insert_space { - result.push(' '); + TokenTree::Token(_, Token::Colon) if self.is_arg => { + self.add_meta_variable(&mut iter); } - let formatted = format_macro_args(d.tts, shape)?; - match d.delim { - DelimToken::Paren => { - result.push_str(&format!("({})", formatted)); - insert_space = SpaceState::Always; - } - DelimToken::Bracket => { - result.push_str(&format!("[{}]", formatted)); - insert_space = SpaceState::Always; - } - DelimToken::Brace => { - result.push_str(&format!(" {{ {} }}", formatted)); - insert_space = SpaceState::Always; + TokenTree::Token(sp, ref t) => self.update_buffer(sp.lo(), t), + TokenTree::Delimited(sp, ref delimited) => { + if !self.buf.is_empty() { + if next_space(&self.last_tok) == SpaceState::Always { + self.add_separator(); + } else { + self.add_other(); + } } - DelimToken::NoDelim => { - result.push_str(&format!("{}", formatted)); - insert_space = SpaceState::Always; + + let mut parser = MacroArgParser::new(); + parser.lo = sp.lo(); + let mut delimited_arg = parser.parse(delimited.tts.clone()); + + if self.is_arg { + // Parse '*' or '+'. + let mut buffer = String::new(); + let mut first = false; + let mut lo = sp.lo(); + + while let Some(ref next_tok) = iter.next() { + self.set_last_tok(next_tok); + if first { + first = false; + lo = next_tok.span().lo(); + } + + match next_tok { + TokenTree::Token(_, Token::BinOp(BinOpToken::Plus)) + | TokenTree::Token(_, Token::Question) + | TokenTree::Token(_, Token::BinOp(BinOpToken::Star)) => { + break; + } + TokenTree::Token(_, ref t) => { + buffer.push_str(&pprust::token_to_string(t)) + } + _ => unreachable!(), + } + } + + let another = if buffer.trim().is_empty() { + None + } else { + Some(Box::new(ParsedMacroArg { + kind: MacroArgKind::Other(buffer, "".to_owned()), + span: mk_sp(lo, self.hi), + })) + }; + + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Repeat( + delimited.delim, + delimited_arg, + another, + self.last_tok.clone(), + ), + span: mk_sp(self.lo, self.hi), + }); + } else { + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Delimited(delimited.delim, delimited_arg), + span: *sp, + }); } } } + + self.set_last_tok(tok); + } + + if !self.buf.is_empty() { + self.add_other(); + } + + self.result + } +} + +fn wrap_macro_args( + context: &RewriteContext, + args: &[ParsedMacroArg], + shape: Shape, +) -> Option { + wrap_macro_args_inner(context, args, shape, false) + .or_else(|| wrap_macro_args_inner(context, args, shape, true)) +} + +fn wrap_macro_args_inner( + context: &RewriteContext, + args: &[ParsedMacroArg], + shape: Shape, + use_multiple_lines: bool, +) -> Option { + let mut result = String::with_capacity(128); + let mut iter = args.iter().peekable(); + let indent_str = shape.indent.to_string_with_newline(context.config); + + while let Some(ref arg) = iter.next() { + let nested_shape = if use_multiple_lines { + shape.with_max_width(context.config) + } else { + shape + }; + result.push_str(&arg.rewrite(context, nested_shape, use_multiple_lines)?); + + if use_multiple_lines && arg.kind.ends_with_space() { + result.pop(); + result.push_str(&indent_str); + } else if let Some(ref next_arg) = iter.peek() { + let space_before_dollar = + !arg.kind.ends_with_space() && next_arg.kind.starts_with_dollar(); + if space_before_dollar { + result.push(' '); + } } } - if result.len() <= shape.width { - Some(result) - } else { + if !use_multiple_lines && result.len() >= shape.width { None + } else { + Some(result) } } +// This is a bit sketchy. The token rules probably need tweaking, but it works +// for some common cases. I hope the basic logic is sufficient. Note that the +// meaning of some tokens is a bit different here from usual Rust, e.g., `*` +// and `(`/`)` have special meaning. +// +// We always try and format on one line. +// FIXME: Use multi-line when every thing does not fit on one line. +fn format_macro_args( + context: &RewriteContext, + toks: ThinTokenStream, + shape: Shape, +) -> Option { + let parsed_args = MacroArgParser::new().parse(toks); + wrap_macro_args(context, &parsed_args, shape) +} + // We should insert a space if the next token is a: -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] enum SpaceState { Never, Punctuation, @@ -538,6 +866,8 @@ enum SpaceState { } fn force_space_before(tok: &Token) -> bool { + debug!("tok: force_space_before {:?}", tok); + match *tok { Token::Eq | Token::Lt @@ -562,13 +892,6 @@ fn force_space_before(tok: &Token) -> bool { } } -fn force_no_space_before(tok: &Token) -> bool { - match *tok { - Token::Semi | Token::Comma | Token::Dot => true, - Token::BinOp(bot) => bot == BinOpToken::Star, - _ => false, - } -} fn ident_like(tok: &Token) -> bool { match *tok { Token::Ident(_) | Token::Literal(..) | Token::Lifetime(_) => true, @@ -577,6 +900,8 @@ fn ident_like(tok: &Token) -> bool { } fn next_space(tok: &Token) -> SpaceState { + debug!("next_space: {:?}", tok); + match *tok { Token::Not | Token::Tilde @@ -804,7 +1129,7 @@ impl MacroBranch { } // 5 = " => {" - let mut result = format_macro_args(self.args.clone(), shape.sub_width(5)?)?; + let mut result = format_macro_args(context, self.args.clone(), shape.sub_width(5)?)?; if multi_branch_style { result += " =>"; From 95507e3a4369404443787ac835b2c377c685472b Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 12:33:30 +0900 Subject: [PATCH 4/9] Remove has_prefix_space --- src/macros.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index d6420af764b..e600b119678 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -519,15 +519,6 @@ impl MacroArgKind { } } - fn has_prefix_space(&self) -> bool { - match *self { - MacroArgKind::Separator(_, ref prefix) | MacroArgKind::Other(_, ref prefix) => { - prefix.starts_with(" ") - } - _ => false, - } - } - fn rewrite( &self, context: &RewriteContext, From 3f7b59ca2b3f3eaa5659743dfa28dce9f7302a74 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 12:33:59 +0900 Subject: [PATCH 5/9] Break before meta variables when using multiple lines --- src/macros.rs | 16 ++++++++++++++-- tests/source/macro_rules.rs | 3 +++ tests/target/macro_rules.rs | 4 ++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index e600b119678..f71c648baac 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -519,6 +519,14 @@ impl MacroArgKind { } } + fn has_meta_var(&self) -> bool { + match *self { + MacroArgKind::MetaVariable(..) => true, + MacroArgKind::Repeat(_, ref args, _, _) => args.iter().any(|a| a.kind.has_meta_var()), + _ => false, + } + } + fn rewrite( &self, context: &RewriteContext, @@ -812,8 +820,12 @@ fn wrap_macro_args_inner( }; result.push_str(&arg.rewrite(context, nested_shape, use_multiple_lines)?); - if use_multiple_lines && arg.kind.ends_with_space() { - result.pop(); + if use_multiple_lines + && (arg.kind.ends_with_space() || iter.peek().map_or(false, |a| a.kind.has_meta_var())) + { + if arg.kind.ends_with_space() { + result.pop(); + } result.push_str(&indent_str); } else if let Some(ref next_arg) = iter.peek() { let space_before_dollar = diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index 17bb3067869..2d0d0e80a6b 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -7,6 +7,9 @@ macro_rules! m { ( $($beginning:ident),*;$middle:ident;$($end:ident),* ) => (); ( $($beginning: ident),*; $middle: ident; $($end: ident),*; $($beginning: ident),*; $middle: ident; $($end: ident),* ) => {}; ( $ name : ident ( $ ( $ dol : tt $ var : ident ) * ) $ ( $ body : tt ) * ) => (); + ( $( $ i : ident : $ ty : ty , $def : expr , $stb : expr , $ ( $ dstring : tt ) , + ) ; + $ ( ; ) * + $( $ i : ident : $ ty : ty , $def : expr , $stb : expr , $ ( $ dstring : tt ) , + ) ; + $ ( ; ) * + ) => {}; } macro_rules! m { diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index 5bedc2a8258..54d7dde6842 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -14,6 +14,10 @@ macro_rules! m { $($end: ident),* ) => {}; ($name: ident($($dol: tt $var: ident)*) $($body: tt)*) => {}; + ( + $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* + $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* + ) => {}; } macro_rules! m { From 0fd174d5f1e51a03b6454617c780f2ea6cd4cb6b Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 13:12:16 +0900 Subject: [PATCH 6/9] Handle binary operators and lifetimes --- src/macros.rs | 18 +++++++++++++----- tests/source/macro_rules.rs | 7 +++++++ tests/target/macro_rules.rs | 7 +++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index f71c648baac..9b5fda2c1ec 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -669,8 +669,16 @@ impl MacroArgParser { if self.buf.is_empty() { self.lo = lo; self.start_tok = t.clone(); - } else if force_space_before(t) { - self.buf.push(' '); + } else { + let needs_space = match next_space(&self.last_tok) { + SpaceState::Ident => ident_like(t), + SpaceState::Punctuation => !ident_like(t), + SpaceState::Always => true, + SpaceState::Never => false, + }; + if force_space_before(t) || needs_space { + self.buf.push(' '); + } } self.buf.push_str(&pprust::token_to_string(t)); @@ -888,9 +896,9 @@ fn force_space_before(tok: &Token) -> bool { | Token::RArrow | Token::LArrow | Token::FatArrow + | Token::BinOp(_) | Token::Pound | Token::Dollar => true, - Token::BinOp(bot) => bot != BinOpToken::Star, _ => false, } } @@ -907,6 +915,7 @@ fn next_space(tok: &Token) -> SpaceState { match *tok { Token::Not + | Token::BinOp(BinOpToken::And) | Token::Tilde | Token::At | Token::Comma @@ -916,8 +925,7 @@ fn next_space(tok: &Token) -> SpaceState { | Token::DotDotEq | Token::DotEq | Token::Question - | Token::Underscore - | Token::BinOp(_) => SpaceState::Punctuation, + | Token::Underscore => SpaceState::Punctuation, Token::ModSep | Token::Pound diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index 2d0d0e80a6b..1a4d8131600 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -10,6 +10,13 @@ macro_rules! m { ( $( $ i : ident : $ ty : ty , $def : expr , $stb : expr , $ ( $ dstring : tt ) , + ) ; + $ ( ; ) * $( $ i : ident : $ ty : ty , $def : expr , $stb : expr , $ ( $ dstring : tt ) , + ) ; + $ ( ; ) * ) => {}; + ( $foo: tt foo [$ attr : meta] $name: ident ) => {}; + ( $foo: tt [$ attr: meta] $name: ident ) => {}; + ( $foo: tt &'a [$attr : meta] $name: ident ) => {}; + ( $foo: tt foo # [ $attr : meta] $name: ident ) => {}; + ( $foo: tt # [ $attr : meta] $name: ident) => {}; + ( $foo: tt &'a # [ $attr : meta] $name: ident ) => {}; + ( $ x : tt foo bar foo bar foo bar $ y : tt => x*y*z $ z : tt , $ ( $a: tt ) , * ) => {}; } macro_rules! m { diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index 54d7dde6842..ece16399184 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -18,6 +18,13 @@ macro_rules! m { $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* ) => {}; + ($foo: tt foo[$attr: meta] $name: ident) => {}; + ($foo: tt[$attr: meta] $name: ident) => {}; + ($foo: tt &'a[$attr: meta] $name: ident) => {}; + ($foo: tt foo #[$attr: meta] $name: ident) => {}; + ($foo: tt #[$attr: meta] $name: ident) => {}; + ($foo: tt &'a #[$attr: meta] $name: ident) => {}; + ($x: tt foo bar foo bar foo bar $y: tt => x * y * z $z: tt, $($a: tt),*) => {}; } macro_rules! m { From 96a83b57e51793de579dc90dc82525cd9f018148 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 13:49:06 +0900 Subject: [PATCH 7/9] Add some doc comments and factor out add_repeat and add_delimited --- src/macros.rs | 148 +++++++++++++++++++++++++++++++------------------- 1 file changed, 91 insertions(+), 57 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 9b5fda2c1ec..9c7a7177ea1 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -465,15 +465,25 @@ fn replace_names(input: &str) -> Option<(String, HashMap)> { #[derive(Debug, Clone)] enum MacroArgKind { + /// e.g. `$x: expr`. MetaVariable(ast::Ident, String), + /// e.g. `$($foo: expr),*` Repeat( + /// `()`, `[]` or `{}`. DelimToken, + /// Inner arguments inside delimiters. Vec, + /// Something after the closing delimiter and the repeat token, if available. Option>, + /// The repeat token. This could be one of `*`, `+` or `?`. Token, ), + /// e.g. `[derive(Debug)]` Delimited(DelimToken, Vec), + /// A possible separator. e.g. `,` or `;`. Separator(String, String), + /// Other random stuff that does not fit to other kinds. + /// e.g. `== foo` in `($x: expr == foo)`. Other(String, String), } @@ -589,13 +599,21 @@ impl ParsedMacroArg { } } +/// Parses macro arguments on macro def. struct MacroArgParser { + /// Holds either a name of the next metavariable, a separator or a junk. + buf: String, + /// The start position on the current buffer. lo: BytePos, + /// The first token of the current buffer. + start_tok: Token, + /// Set to true if we are parsing a metavariable or a repeat. + is_meta_var: bool, + /// The position of the last token. hi: BytePos, - buf: String, - is_arg: bool, + /// The last token parsed. last_tok: Token, - start_tok: Token, + /// Holds the parsed arguments. result: Vec, } @@ -612,7 +630,7 @@ impl MacroArgParser { lo: BytePos(0), hi: BytePos(0), buf: String::new(), - is_arg: false, + is_meta_var: false, last_tok: Token::Eof, start_tok: Token::Eof, result: vec![], @@ -659,12 +677,70 @@ impl MacroArgParser { }); self.buf.clear(); - self.is_arg = false; + self.is_meta_var = false; } _ => unreachable!(), } } + fn add_delimited(&mut self, inner: Vec, delim: DelimToken, span: Span) { + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Delimited(delim, inner), + span, + }); + } + + // $($foo: expr),? + fn add_repeat( + &mut self, + inner: Vec, + delim: DelimToken, + iter: &mut Cursor, + span: Span, + ) { + let mut buffer = String::new(); + let mut first = false; + let mut lo = span.lo(); + let mut hi = span.hi(); + + // Parse '*', '+' or '?. + while let Some(ref tok) = iter.next() { + self.set_last_tok(tok); + if first { + first = false; + lo = tok.span().lo(); + } + + match tok { + TokenTree::Token(_, Token::BinOp(BinOpToken::Plus)) + | TokenTree::Token(_, Token::Question) + | TokenTree::Token(_, Token::BinOp(BinOpToken::Star)) => { + break; + } + TokenTree::Token(sp, ref t) => { + buffer.push_str(&pprust::token_to_string(t)); + hi = sp.hi(); + } + _ => unreachable!(), + } + } + + // There could be some random stuff between ')' and '*', '+' or '?'. + let another = if buffer.trim().is_empty() { + None + } else { + Some(Box::new(ParsedMacroArg { + kind: MacroArgKind::Other(buffer, "".to_owned()), + span: mk_sp(lo, hi), + })) + }; + + self.result.push(ParsedMacroArg { + kind: MacroArgKind::Repeat(delim, inner, another, self.last_tok.clone()), + span: mk_sp(self.lo, self.hi), + }); + } + fn update_buffer(&mut self, lo: BytePos, t: &Token) { if self.buf.is_empty() { self.lo = lo; @@ -716,15 +792,15 @@ impl MacroArgParser { } // Start keeping the name of this metavariable in the buffer. - self.is_arg = true; + self.is_meta_var = true; self.lo = sp.lo(); self.start_tok = Token::Dollar; } - TokenTree::Token(_, Token::Colon) if self.is_arg => { + TokenTree::Token(_, Token::Colon) if self.is_meta_var => { self.add_meta_variable(&mut iter); } TokenTree::Token(sp, ref t) => self.update_buffer(sp.lo(), t), - TokenTree::Delimited(sp, ref delimited) => { + TokenTree::Delimited(sp, delimited) => { if !self.buf.is_empty() { if next_space(&self.last_tok) == SpaceState::Always { self.add_separator(); @@ -733,59 +809,15 @@ impl MacroArgParser { } } + // Parse the stuff inside delimiters. let mut parser = MacroArgParser::new(); parser.lo = sp.lo(); - let mut delimited_arg = parser.parse(delimited.tts.clone()); - - if self.is_arg { - // Parse '*' or '+'. - let mut buffer = String::new(); - let mut first = false; - let mut lo = sp.lo(); - - while let Some(ref next_tok) = iter.next() { - self.set_last_tok(next_tok); - if first { - first = false; - lo = next_tok.span().lo(); - } - - match next_tok { - TokenTree::Token(_, Token::BinOp(BinOpToken::Plus)) - | TokenTree::Token(_, Token::Question) - | TokenTree::Token(_, Token::BinOp(BinOpToken::Star)) => { - break; - } - TokenTree::Token(_, ref t) => { - buffer.push_str(&pprust::token_to_string(t)) - } - _ => unreachable!(), - } - } + let delimited_arg = parser.parse(delimited.tts.clone()); - let another = if buffer.trim().is_empty() { - None - } else { - Some(Box::new(ParsedMacroArg { - kind: MacroArgKind::Other(buffer, "".to_owned()), - span: mk_sp(lo, self.hi), - })) - }; - - self.result.push(ParsedMacroArg { - kind: MacroArgKind::Repeat( - delimited.delim, - delimited_arg, - another, - self.last_tok.clone(), - ), - span: mk_sp(self.lo, self.hi), - }); + if self.is_meta_var { + self.add_repeat(delimited_arg, delimited.delim, &mut iter, *sp); } else { - self.result.push(ParsedMacroArg { - kind: MacroArgKind::Delimited(delimited.delim, delimited_arg), - span: *sp, - }); + self.add_delimited(delimited_arg, delimited.delim, *sp); } } } @@ -793,6 +825,8 @@ impl MacroArgParser { self.set_last_tok(tok); } + // We are left with some stuff in the buffer. Since there is nothing + // left to separate, add this as `Other`. if !self.buf.is_empty() { self.add_other(); } From f8109f8e9c9a460dcb3176bd002985d8a27afc32 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 14:08:24 +0900 Subject: [PATCH 8/9] Put spaces around braces --- src/macros.rs | 19 ++++++++------ tests/source/macro_rules.rs | 20 +++++++++++++++ tests/target/macro_rules.rs | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 9c7a7177ea1..e36b4c51993 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -496,7 +496,7 @@ fn delim_token_to_str( let (lhs, rhs) = match *delim_token { DelimToken::Paren => ("(", ")"), DelimToken::Bracket => ("[", "]"), - DelimToken::Brace => ("{", "}"), + DelimToken::Brace => ("{ ", " }"), DelimToken::NoDelim => ("", ""), }; if use_multiple_lines { @@ -515,6 +515,13 @@ fn delim_token_to_str( } impl MacroArgKind { + fn starts_with_brace(&self) -> bool { + match *self { + MacroArgKind::Repeat(DelimToken::Brace, _, _, _) + | MacroArgKind::Delimited(DelimToken::Brace, _) => true, + _ => false, + } + } fn starts_with_dollar(&self) -> bool { match *self { MacroArgKind::Repeat(..) | MacroArgKind::MetaVariable(..) => true, @@ -855,12 +862,7 @@ fn wrap_macro_args_inner( let indent_str = shape.indent.to_string_with_newline(context.config); while let Some(ref arg) = iter.next() { - let nested_shape = if use_multiple_lines { - shape.with_max_width(context.config) - } else { - shape - }; - result.push_str(&arg.rewrite(context, nested_shape, use_multiple_lines)?); + result.push_str(&arg.rewrite(context, shape, use_multiple_lines)?); if use_multiple_lines && (arg.kind.ends_with_space() || iter.peek().map_or(false, |a| a.kind.has_meta_var())) @@ -872,7 +874,8 @@ fn wrap_macro_args_inner( } else if let Some(ref next_arg) = iter.peek() { let space_before_dollar = !arg.kind.ends_with_space() && next_arg.kind.starts_with_dollar(); - if space_before_dollar { + let space_before_brace = next_arg.kind.starts_with_brace(); + if space_before_dollar || space_before_brace { result.push(' '); } } diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index 1a4d8131600..de927afc831 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -19,6 +19,26 @@ macro_rules! m { ( $ x : tt foo bar foo bar foo bar $ y : tt => x*y*z $ z : tt , $ ( $a: tt ) , * ) => {}; } + +macro_rules! impl_a_method { + ($n:ident ( $a:ident : $ta:ty ) -> $ret:ty { $body:expr }) => { + fn $n($a:$ta) -> $ret { $body } + macro_rules! $n { ($va:expr) => { $n($va) } } + }; + ($n:ident ( $a:ident : $ta:ty, $b:ident : $tb:ty ) -> $ret:ty { $body:expr }) => { + fn $n($a:$ta, $b:$tb) -> $ret { $body } + macro_rules! $n { ($va:expr, $vb:expr) => { $n($va, $vb) } } + }; + ($n:ident ( $a:ident : $ta:ty, $b:ident : $tb:ty, $c:ident : $tc:ty ) -> $ret:ty { $body:expr }) => { + fn $n($a:$ta, $b:$tb, $c:$tc) -> $ret { $body } + macro_rules! $n { ($va:expr, $vb:expr, $vc:expr) => { $n($va, $vb, $vc) } } + }; + ($n:ident ( $a:ident : $ta:ty, $b:ident : $tb:ty, $c:ident : $tc:ty, $d:ident : $td:ty ) -> $ret:ty { $body:expr }) => { + fn $n($a:$ta, $b:$tb, $c:$tc, $d:$td) -> $ret { $body } + macro_rules! $n { ($va:expr, $vb:expr, $vc:expr, $vd:expr) => { $n($va, $vb, $vc, $vd) } } + }; +} + macro_rules! m { // a ($expr :expr, $( $func : ident ) * ) => { diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index ece16399184..22fed940685 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -27,6 +27,55 @@ macro_rules! m { ($x: tt foo bar foo bar foo bar $y: tt => x * y * z $z: tt, $($a: tt),*) => {}; } +macro_rules! impl_a_method { + ($n: ident($a: ident: $ta: ty) -> $ret: ty { $body: expr }) => { + fn $n($a: $ta) -> $ret { + $body + } + macro_rules! $n { + ($va: expr) => { + $n($va) + }; + } + }; + ($n: ident($a: ident: $ta: ty, $b: ident: $tb: ty) -> $ret: ty { $body: expr }) => { + fn $n($a: $ta, $b: $tb) -> $ret { + $body + } + macro_rules! $n { + ($va: expr,$vb: expr) => { + $n($va, $vb) + }; + } + }; + ( + $n: ident($a: ident: $ta: ty, $b: ident: $tb: ty, $c: ident: $tc: ty) -> + $ret: ty { $body: expr } + ) => { + fn $n($a: $ta, $b: $tb, $c: $tc) -> $ret { + $body + } + macro_rules! $n { + ($va: expr,$vb: expr,$vc: expr) => { + $n($va, $vb, $vc) + }; + } + }; + ( + $n: ident($a: ident: $ta: ty, $b: ident: $tb: ty, $c: ident: $tc: ty, $d: ident: $td: ty) -> + $ret: ty { $body: expr } + ) => { + fn $n($a: $ta, $b: $tb, $c: $tc, $d: $td) -> $ret { + $body + } + macro_rules! $n { + ($va: expr,$vb: expr,$vc: expr,$vd: expr) => { + $n($va, $vb, $vc, $vd) + }; + } + }; +} + macro_rules! m { // a ($expr: expr, $($func: ident)*) => {{ From adc257f4b3b2a9a971dfd19721d9ad97fae5d37a Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sun, 18 Mar 2018 14:29:36 +0900 Subject: [PATCH 9/9] Put a space before colon that appears after a meta variable Closes #2534. --- src/macros.rs | 4 ++++ tests/source/macro_rules.rs | 6 ++++++ tests/target/macro_rules.rs | 18 ++++++++++++------ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index e36b4c51993..784e6c5798b 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -522,6 +522,7 @@ impl MacroArgKind { _ => false, } } + fn starts_with_dollar(&self) -> bool { match *self { MacroArgKind::Repeat(..) | MacroArgKind::MetaVariable(..) => true, @@ -777,6 +778,9 @@ impl MacroArgParser { if ident_like(&self.start_tok) { return true; } + if self.start_tok == Token::Colon { + return true; + } } if force_space_before(&self.start_tok) { diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index de927afc831..f390b426317 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -147,6 +147,12 @@ macro foo($type_name: ident, $docs: expr) { pub struct $type_name; } +// #2534 +macro_rules! foo { + ($a:ident : $b:ty) => {}; + ($a:ident $b:ident $c:ident) => {}; +} + // #2538 macro_rules! add_message_to_notes { ($msg:expr) => {{ diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index 22fed940685..793efcb6b0c 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -15,8 +15,8 @@ macro_rules! m { ) => {}; ($name: ident($($dol: tt $var: ident)*) $($body: tt)*) => {}; ( - $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* - $($i: ident: $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* + $($i: ident : $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* + $($i: ident : $ty: ty, $def: expr, $stb: expr, $($dstring: tt),+);+ $(;)* ) => {}; ($foo: tt foo[$attr: meta] $name: ident) => {}; ($foo: tt[$attr: meta] $name: ident) => {}; @@ -28,7 +28,7 @@ macro_rules! m { } macro_rules! impl_a_method { - ($n: ident($a: ident: $ta: ty) -> $ret: ty { $body: expr }) => { + ($n: ident($a: ident : $ta: ty) -> $ret: ty { $body: expr }) => { fn $n($a: $ta) -> $ret { $body } @@ -38,7 +38,7 @@ macro_rules! impl_a_method { }; } }; - ($n: ident($a: ident: $ta: ty, $b: ident: $tb: ty) -> $ret: ty { $body: expr }) => { + ($n: ident($a: ident : $ta: ty, $b: ident : $tb: ty) -> $ret: ty { $body: expr }) => { fn $n($a: $ta, $b: $tb) -> $ret { $body } @@ -49,7 +49,7 @@ macro_rules! impl_a_method { } }; ( - $n: ident($a: ident: $ta: ty, $b: ident: $tb: ty, $c: ident: $tc: ty) -> + $n: ident($a: ident : $ta: ty, $b: ident : $tb: ty, $c: ident : $tc: ty) -> $ret: ty { $body: expr } ) => { fn $n($a: $ta, $b: $tb, $c: $tc) -> $ret { @@ -62,7 +62,7 @@ macro_rules! impl_a_method { } }; ( - $n: ident($a: ident: $ta: ty, $b: ident: $tb: ty, $c: ident: $tc: ty, $d: ident: $td: ty) -> + $n: ident($a: ident : $ta: ty, $b: ident : $tb: ty, $c: ident : $tc: ty, $d: ident : $td: ty) -> $ret: ty { $body: expr } ) => { fn $n($a: $ta, $b: $tb, $c: $tc, $d: $td) -> $ret { @@ -180,6 +180,12 @@ macro foo($type_name: ident, $docs: expr) { pub struct $type_name; } +// #2534 +macro_rules! foo { + ($a: ident : $b: ty) => {}; + ($a: ident $b: ident $c: ident) => {}; +} + // #2538 macro_rules! add_message_to_notes { ($msg: expr) => {{