From 5e59e686c175eff36d2292205bfca3583d599227 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Thu, 7 Jun 2018 12:30:14 +0900 Subject: [PATCH 1/5] Add tests for #2607 and #2770 --- tests/source/macro_rules.rs | 27 +++++++++++++++++++++++++++ tests/target/macro_rules.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/tests/source/macro_rules.rs b/tests/source/macro_rules.rs index 5ed5f905883..fdcde7f6f59 100644 --- a/tests/source/macro_rules.rs +++ b/tests/source/macro_rules.rs @@ -204,3 +204,30 @@ macro_rules! foo { macro_rules! __wundergraph_expand_sqlite_mutation { ( $mutation_name:ident $((context = $($context:tt)*))*{ $( $entity_name:ident( $(insert = $insert:ident,)* $(update = $update:ident,)* $(delete = $($delete:tt)+)* ), )* } ) => {}; } + +// #2607 +macro_rules! bench { + ($ty:ident) => { + criterion_group!( + name = benches; + config = ::common_bench::reduced_samples(); + targets = call, map; + ); + }; +} + +// #2770 +macro_rules! save_regs { + () => { + asm!("push rax + push rcx + push rdx + push rsi + push rdi + push r8 + push r9 + push r10 + push r11" + :::: "intel", "volatile"); + }; +} diff --git a/tests/target/macro_rules.rs b/tests/target/macro_rules.rs index c366d75cf91..66790091f86 100644 --- a/tests/target/macro_rules.rs +++ b/tests/target/macro_rules.rs @@ -246,3 +246,30 @@ macro_rules! __wundergraph_expand_sqlite_mutation { } ) => {}; } + +// #2607 +macro_rules! bench { + ($ty:ident) => { + criterion_group!( + name = benches; + config = ::common_bench::reduced_samples(); + targets = call, map; + ); + }; +} + +// #2770 +macro_rules! save_regs { + () => { + asm!("push rax + push rcx + push rdx + push rsi + push rdi + push r8 + push r9 + push r10 + push r11" + :::: "intel", "volatile"); + }; +} From 94e68b1eb6c0407f46e6ebffda6cd908b5fa47f5 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Thu, 7 Jun 2018 12:32:10 +0900 Subject: [PATCH 2/5] Set the flag in RewriteContext when rewriting macro call failed --- src/macros.rs | 17 +++++++++++++---- src/rewrite.rs | 2 ++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index ae95acaf301..674a652322d 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -129,6 +129,12 @@ fn rewrite_macro_name(path: &ast::Path, extra_ident: Option) -> Stri } } +// Use this on failing to format the macro call. +fn return_original_snippet_with_failure_marked(context: &RewriteContext, span: Span) -> Option { + context.macro_rewrite_failure.replace(true); + Some(context.snippet(span).to_owned()) +} + pub fn rewrite_macro( mac: &ast::Mac, extra_ident: Option, @@ -138,6 +144,9 @@ pub fn rewrite_macro( ) -> Option { context.inside_macro.replace(true); let result = rewrite_macro_inner(mac, extra_ident, context, shape, position); + if result.is_none() { + context.macro_rewrite_failure.replace(true); + } context.inside_macro.replace(false); result } @@ -196,7 +205,7 @@ pub fn rewrite_macro_inner( loop { match parse_macro_arg(&mut parser) { Some(arg) => arg_vec.push(arg), - None => return Some(context.snippet(mac.span).to_owned()), + None => return return_original_snippet_with_failure_marked(context, mac.span), } match parser.token { @@ -216,13 +225,13 @@ pub fn rewrite_macro_inner( break; } } - None => return Some(context.snippet(mac.span).to_owned()), + None => return return_original_snippet_with_failure_marked(context, mac.span), } } } - return Some(context.snippet(mac.span).to_owned()); + return return_original_snippet_with_failure_marked(context, mac.span); } - _ => return Some(context.snippet(mac.span).to_owned()), + _ => return return_original_snippet_with_failure_marked(context, mac.span), } parser.bump(); diff --git a/src/rewrite.rs b/src/rewrite.rs index 00c03a3c879..90b613df6c4 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -39,6 +39,8 @@ pub struct RewriteContext<'a> { // When rewriting chain, veto going multi line except the last element pub force_one_line_chain: RefCell, pub snippet_provider: &'a SnippetProvider<'a>, + // Used for `format_snippet` + pub(crate) macro_rewrite_failure: RefCell, pub(crate) report: FormatReport, } From d1477ca1de6ff7b0d5ad0fe190ed9ae7fe65f1f5 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Thu, 7 Jun 2018 12:32:58 +0900 Subject: [PATCH 3/5] Add a field in Summary for notiyfing about formatting failure of macro --- src/config/summary.rs | 11 +++++++++++ src/lib.rs | 13 ++++++++++--- src/visitor.rs | 19 ++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/config/summary.rs b/src/config/summary.rs index 53d2c0a2f20..1ef6d18a540 100644 --- a/src/config/summary.rs +++ b/src/config/summary.rs @@ -23,6 +23,9 @@ pub struct Summary { // Code is valid, but it is impossible to format it properly. has_formatting_errors: bool, + // Code contains macro call that was unable to format. + pub(crate) has_macro_format_failure: bool, + // Failed a check, such as the license check or other opt-in checking. has_check_errors: bool, @@ -80,6 +83,10 @@ impl Summary { self.has_check_errors } + pub(crate) fn has_macro_formatting_failure(&self) -> bool { + self.has_macro_format_failure + } + pub fn add_operational_error(&mut self) { self.has_operational_errors = true; } @@ -100,6 +107,10 @@ impl Summary { self.has_diff = true; } + pub(crate) fn add_macro_foramt_failure(&mut self) { + self.has_macro_format_failure = true; + } + pub fn has_no_errors(&self) -> bool { !(self.has_operational_errors || self.has_parsing_errors diff --git a/src/lib.rs b/src/lib.rs index 02307a8dc6d..a8f09e4843d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -422,13 +422,14 @@ fn format_ast( config: &Config, report: FormatReport, mut after_file: F, -) -> Result<(FileMap, bool), io::Error> +) -> Result<(FileMap, bool, bool), io::Error> where F: FnMut(&FileName, &mut String, &[(usize, usize)], &FormatReport) -> Result, { let mut result = FileMap::new(); // diff mode: check if any files are differing let mut has_diff = false; + let mut has_macro_rewrite_failure = false; let skip_children = config.skip_children(); for (path, module) in modules::list_files(krate, parse_session.codemap())? { @@ -472,10 +473,12 @@ where } }; + has_macro_rewrite_failure |= visitor.macro_rewrite_failure; + result.push((path.clone(), visitor.buffer)); } - Ok((result, has_diff)) + Ok((result, has_diff, has_macro_rewrite_failure)) } /// Returns true if the line with the given line number was skipped by `#[rustfmt::skip]`. @@ -902,7 +905,7 @@ fn format_input_inner( } match format_result { - Ok((file_map, has_diff)) => { + Ok((file_map, has_diff, has_macro_rewrite_failure)) => { if report.has_warnings() { summary.add_formatting_error(); } @@ -911,6 +914,10 @@ fn format_input_inner( summary.add_diff(); } + if has_macro_rewrite_failure { + summary.add_macro_foramt_failure(); + } + Ok((summary, file_map, report)) } Err(e) => Err((From::from(e), summary)), diff --git a/src/visitor.rs b/src/visitor.rs index 4a0ce4399a4..830f3079aa8 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -70,6 +70,7 @@ pub struct FmtVisitor<'a> { pub snippet_provider: &'a SnippetProvider<'a>, pub line_number: usize, pub skipped_range: Vec<(usize, usize)>, + pub macro_rewrite_failure: bool, pub(crate) report: FormatReport, } @@ -519,7 +520,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // 1 = ; let shape = self.shape().sub_width(1).unwrap(); - let rewrite = rewrite_macro(mac, ident, &self.get_context(), shape, pos); + let rewrite = self.with_context(|ctx| rewrite_macro(mac, ident, ctx, shape, pos)); self.push_rewrite(mac.span, rewrite); } @@ -578,6 +579,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { snippet_provider, line_number: 0, skipped_range: vec![], + macro_rewrite_failure: false, report, } } @@ -736,6 +738,20 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } } + pub fn with_context(&mut self, f: F) -> Option + where + F: Fn(&RewriteContext) -> Option, + { + let mut result; + let macro_rewrite_failure = { + let context = self.get_context(); + result = f(&context); + unsafe { *context.macro_rewrite_failure.as_ptr() } + }; + self.macro_rewrite_failure |= macro_rewrite_failure; + result + } + pub fn get_context(&self) -> RewriteContext { RewriteContext { parse_session: self.parse_session, @@ -746,6 +762,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { is_if_else_block: RefCell::new(false), force_one_line_chain: RefCell::new(false), snippet_provider: self.snippet_provider, + macro_rewrite_failure: RefCell::new(false), report: self.report.clone(), } } From c95fa8cbe22d782cd7bc007dfe06aba9ceddff78 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Thu, 7 Jun 2018 12:33:33 +0900 Subject: [PATCH 4/5] Return None when the formatting of macro failed in format_snippet --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index a8f09e4843d..8d4cbb51b5a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -689,6 +689,7 @@ fn format_snippet(snippet: &str, config: &Config) -> Option { config.set().hide_parse_errors(true); match format_input(input, &config, Some(&mut out)) { // `format_input()` returns an empty string on parsing error. + Ok((summary, _)) if summary.has_macro_formatting_failure() => None, Ok(..) if out.is_empty() && !snippet.is_empty() => None, Ok(..) => String::from_utf8(out).ok(), Err(..) => None, From 19054347cad5b0edc006957efd92188671eb0735 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Thu, 7 Jun 2018 15:20:01 +0900 Subject: [PATCH 5/5] Fix test failures --- src/macros.rs | 11 +++++++++-- src/visitor.rs | 2 +- tests/target/issue-2523.rs | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 674a652322d..9053aaebd90 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -130,7 +130,10 @@ fn rewrite_macro_name(path: &ast::Path, extra_ident: Option) -> Stri } // Use this on failing to format the macro call. -fn return_original_snippet_with_failure_marked(context: &RewriteContext, span: Span) -> Option { +fn return_original_snippet_with_failure_marked( + context: &RewriteContext, + span: Span, +) -> Option { context.macro_rewrite_failure.replace(true); Some(context.snippet(span).to_owned()) } @@ -225,7 +228,11 @@ pub fn rewrite_macro_inner( break; } } - None => return return_original_snippet_with_failure_marked(context, mac.span), + None => { + return return_original_snippet_with_failure_marked( + context, mac.span, + ) + } } } } diff --git a/src/visitor.rs b/src/visitor.rs index 830f3079aa8..7d676770421 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -742,7 +742,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { where F: Fn(&RewriteContext) -> Option, { - let mut result; + let result; let macro_rewrite_failure = { let context = self.get_context(); result = f(&context); diff --git a/tests/target/issue-2523.rs b/tests/target/issue-2523.rs index 6805f7ec2ca..d908831c21c 100644 --- a/tests/target/issue-2523.rs +++ b/tests/target/issue-2523.rs @@ -2,7 +2,7 @@ // Do not unindent macro calls in comment with unformattable syntax. //! ```rust -//! let x = 3; +//! let x = 3 ; //! some_macro!(pub fn fn foo() ( //! println!("Don't unindent me!"); //! ));