Merge pull request rust-lang#3001 from scampi/issue2977

propagate errors about failing to rewrite a macro

src/formatting.rs

@@ -175,7 +175,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
         }
 
         self.handler
-            .handle_formatted_file(path, visitor.buffer, &mut self.report)
+            .handle_formatted_file(path, visitor.buffer.to_owned(), &mut self.report)
     }
 }
 

src/macros.rs

@@ -69,7 +69,7 @@ impl Rewrite for ast::Item {
         visitor.block_indent = shape.indent;
         visitor.last_pos = self.span().lo();
         visitor.visit_item(self);
-        Some(visitor.buffer)
+        Some(visitor.buffer.to_owned())
     }
 }
 
@@ -406,7 +406,15 @@ pub fn rewrite_macro_def(
         ";",
         |branch| branch.span.lo(),
         |branch| branch.span.hi(),
-        |branch| branch.rewrite(context, arm_shape, multi_branch_style),
+        |branch| match branch.rewrite(context, arm_shape, multi_branch_style) {
+            Some(v) => Some(v),
+            // if the rewrite returned None because a macro could not be rewritten, then return the
+            // original body
+            None if *context.macro_rewrite_failure.borrow() == true => {
+                Some(context.snippet(branch.body).trim().to_string())
+            }
+            None => None,
+        },
         context.snippet_provider.span_after(span, "{"),
         span.hi(),
         false,

src/visitor.rs

@@ -60,6 +60,7 @@ impl<'a> SnippetProvider<'a> {
 }
 
 pub struct FmtVisitor<'a> {
+    parent_context: Option<&'a RewriteContext<'a>>,
     pub parse_session: &'a ParseSess,
     pub source_map: &'a SourceMap,
     pub buffer: String,
@@ -75,7 +76,21 @@ pub struct FmtVisitor<'a> {
     pub(crate) report: FormatReport,
 }
 
+impl<'a> Drop for FmtVisitor<'a> {
+    fn drop(&mut self) {
+        if let Some(ctx) = self.parent_context {
+            if self.macro_rewrite_failure {
+                ctx.macro_rewrite_failure.replace(true);
+            }
+        }
+    }
+}
+
 impl<'b, 'a: 'b> FmtVisitor<'a> {
+    fn set_parent_context(&mut self, context: &'a RewriteContext) {
+        self.parent_context = Some(context);
+    }
+
     pub fn shape(&self) -> Shape {
         Shape::indented(self.block_indent, self.config)
     }
@@ -97,7 +112,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 if contains_skip(get_attrs_from_stmt(stmt)) {
                     self.push_skipped_with_span(stmt.span());
                 } else {
-                    let rewrite = stmt.rewrite(&self.get_context(), self.shape());
+                    let shape = self.shape().clone();
+                    let rewrite = self.with_context(|ctx| stmt.rewrite(&ctx, shape));
                     self.push_rewrite(stmt.span(), rewrite)
                 }
             }
@@ -350,11 +366,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 let where_span_end = snippet
                     .find_uncommented("{")
                     .map(|x| BytePos(x as u32) + source!(self, item.span).lo());
-                let rw = format_impl(&self.get_context(), item, self.block_indent, where_span_end);
+                let block_indent = self.block_indent.clone();
+                let rw =
+                    self.with_context(|ctx| format_impl(&ctx, item, block_indent, where_span_end));
                 self.push_rewrite(item.span, rw);
             }
             ast::ItemKind::Trait(..) => {
-                let rw = format_trait(&self.get_context(), item, self.block_indent);
+                let block_indent = self.block_indent.clone();
+                let rw = self.with_context(|ctx| format_trait(&ctx, item, block_indent));
                 self.push_rewrite(item.span, rw);
             }
             ast::ItemKind::TraitAlias(ref generics, ref generic_bounds) => {
@@ -575,12 +594,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
     }
 
     pub fn from_context(ctx: &'a RewriteContext) -> FmtVisitor<'a> {
-        FmtVisitor::from_source_map(
+        let mut visitor = FmtVisitor::from_source_map(
             ctx.parse_session,
             ctx.config,
             ctx.snippet_provider,
             ctx.report.clone(),
-        )
+        );
+        visitor.set_parent_context(ctx);
+        visitor
     }
 
     pub(crate) fn from_source_map(
@@ -590,6 +611,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         report: FormatReport,
     ) -> FmtVisitor<'a> {
         FmtVisitor {
+            parent_context: None,
             parse_session,
             source_map: parse_session.source_map(),
             buffer: String::with_capacity(snippet_provider.big_snippet.len() * 2),

tests/source/issue-2977/impl.rs

@@ -0,0 +1,44 @@
+macro_rules! atomic_bits {
+    // the println macro cannot be rewritten because of the asm macro
+    ($type:ty, $ldrex:expr, $strex:expr) => {
+        impl AtomicBits for $type {
+            unsafe fn load_excl(address: usize) -> Self {
+                let raw: $type;
+                asm!($ldrex
+                     : "=r"(raw)
+                     : "r"(address)
+                     :
+                     : "volatile");
+                raw
+            }
+
+            unsafe fn store_excl(self, address: usize) -> bool {
+                let status: $type;
+                println!("{}",
+                         status);
+                status == 0
+            }
+        }
+    };
+
+    // the println macro should be rewritten here
+    ($type:ty) => {
+        fn some_func(self) {
+            let status: $type;
+            println!("{}", status);
+        }
+    };
+
+    // unrewritale macro in func
+    ($type:ty, $ldrex:expr) => {
+        unsafe fn load_excl(address: usize) -> Self {
+            let raw: $type;
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+            raw
+        }
+    }
+}

tests/source/issue-2977/trait.rs

@@ -0,0 +1,44 @@
+macro_rules! atomic_bits {
+    // the println macro cannot be rewritten because of the asm macro
+    ($type:ty, $ldrex:expr, $strex:expr) => {
+        trait $type {
+            unsafe fn load_excl(address: usize) -> Self {
+                let raw: $type;
+                asm!($ldrex
+                     : "=r"(raw)
+                     : "r"(address)
+                     :
+                     : "volatile");
+                raw
+            }
+
+            unsafe fn store_excl(self, address: usize) -> bool {
+                let status: $type;
+                println!("{}",
+                         status);
+                status == 0
+            }
+        }
+    };
+
+    // the println macro should be rewritten here
+    ($type:ty) => {
+        fn some_func(self) {
+            let status: $type;
+            println!("{}", status);
+        }
+    };
+
+    // unrewritale macro in func
+    ($type:ty, $ldrex:expr) => {
+        unsafe fn load_excl(address: usize) -> Self {
+            let raw: $type;
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+            raw
+        }
+    }
+}

tests/target/issue-2977/block.rs

@@ -0,0 +1,11 @@
+macro_rules! atomic_bits {
+    ($ldrex:expr) => {
+        execute(|| {
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+        })
+    };
+}

tests/target/issue-2977/impl.rs

@@ -0,0 +1,44 @@
+macro_rules! atomic_bits {
+    // the println macro cannot be rewritten because of the asm macro
+    ($type:ty, $ldrex:expr, $strex:expr) => {
+        impl AtomicBits for $type {
+            unsafe fn load_excl(address: usize) -> Self {
+                let raw: $type;
+                asm!($ldrex
+                     : "=r"(raw)
+                     : "r"(address)
+                     :
+                     : "volatile");
+                raw
+            }
+
+            unsafe fn store_excl(self, address: usize) -> bool {
+                let status: $type;
+                println!("{}",
+                         status);
+                status == 0
+            }
+        }
+    };
+
+    // the println macro should be rewritten here
+    ($type:ty) => {
+        fn some_func(self) {
+            let status: $type;
+            println!("{}", status);
+        }
+    };
+
+    // unrewritale macro in func
+    ($type:ty, $ldrex:expr) => {
+        unsafe fn load_excl(address: usize) -> Self {
+            let raw: $type;
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+            raw
+        }
+    }
+}

tests/target/issue-2977/item.rs

@@ -0,0 +1,11 @@
+macro_rules! atomic_bits {
+    ($ldrex:expr) => {
+        some_macro!(pub fn foo() {
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+        })
+    };
+}

tests/target/issue-2977/trait.rs

@@ -0,0 +1,44 @@
+macro_rules! atomic_bits {
+    // the println macro cannot be rewritten because of the asm macro
+    ($type:ty, $ldrex:expr, $strex:expr) => {
+        trait $type {
+            unsafe fn load_excl(address: usize) -> Self {
+                let raw: $type;
+                asm!($ldrex
+                     : "=r"(raw)
+                     : "r"(address)
+                     :
+                     : "volatile");
+                raw
+            }
+
+            unsafe fn store_excl(self, address: usize) -> bool {
+                let status: $type;
+                println!("{}",
+                         status);
+                status == 0
+            }
+        }
+    };
+
+    // the println macro should be rewritten here
+    ($type:ty) => {
+        fn some_func(self) {
+            let status: $type;
+            println!("{}", status);
+        }
+    };
+
+    // unrewritale macro in func
+    ($type:ty, $ldrex:expr) => {
+        unsafe fn load_excl(address: usize) -> Self {
+            let raw: $type;
+            asm!($ldrex
+                 : "=r"(raw)
+                 : "r"(address)
+                 :
+                 : "volatile");
+            raw
+        }
+    }
+}