Replace local backtrace with def-use, repair std macro spans

rust-lang · bors · Aug 18, 2016 · Aug 17, 2016 · Aug 17, 2016 · Aug 17, 2016
commit 61865384b8fa6d79d2b36cbd7c899eaf15f4aeea
diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
@@ -12,7 +12,7 @@ use self::Destination::*;
 
 use syntax_pos::{COMMAND_LINE_SP, DUMMY_SP, FileMap, Span, MultiSpan, CharPos};
 
-use {Level, CodeSuggestion, DiagnosticBuilder, CodeMapper};
+use {Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, CodeMapper};
 use RenderSpan::*;
 use snippet::{StyledString, Style, Annotation, Line};
 use styled_buffer::StyledBuffer;
@@ -30,7 +30,10 @@ pub trait Emitter {
 
 impl Emitter for EmitterWriter {
     fn emit(&mut self, db: &DiagnosticBuilder) {
-        self.emit_messages_default(db);
+        let mut primary_span = db.span.clone();
+        let mut children = db.children.clone();
+        self.fix_multispans_in_std_macros(&mut primary_span, &mut children);
+        self.emit_messages_default(&db.level, &db.message, &db.code, &primary_span, &children);
     }
 }
 
@@ -381,19 +384,100 @@ impl EmitterWriter {
         max
     }
 
-    fn get_max_line_num(&mut self, db: &DiagnosticBuilder) -> usize {
+    fn get_max_line_num(&mut self, span: &MultiSpan, children: &Vec<SubDiagnostic>) -> usize {
         let mut max = 0;
 
-        let primary = self.get_multispan_max_line_num(&db.span);
+        let primary = self.get_multispan_max_line_num(span);
         max = if primary > max { primary } else { max };
 
-        for sub in &db.children {
+        for sub in children {
             let sub_result = self.get_multispan_max_line_num(&sub.span);
             max = if sub_result > max { primary } else { max };
         }
         max
     }
 
+    // This "fixes" MultiSpans that contain Spans that are pointing to locations inside of
+    // <*macros>. Since these locations are often difficult to read, we move these Spans from
+    // <*macros> to their corresponding use site.
+    fn fix_multispan_in_std_macros(&mut self, span: &mut MultiSpan) -> bool {
+        let mut spans_updated = false;
+
+        if let Some(ref cm) = self.cm {
+            let mut before_after: Vec<(Span, Span)> = vec![];
+            let mut new_labels: Vec<(Span, String)> = vec![];
+
+            // First, find all the spans in <*macros> and point instead at their use site
+            for sp in span.primary_spans() {
+                if (*sp == COMMAND_LINE_SP) || (*sp == DUMMY_SP) {
+                    continue;
+                }
+                if cm.span_to_filename(sp.clone()).contains("macros>") {
+                    let v = cm.macro_backtrace(sp.clone());
+                    if let Some(use_site) = v.last() {
+                        before_after.push((sp.clone(), use_site.call_site.clone()));
+                    }
+                }
+                for trace in cm.macro_backtrace(sp.clone()).iter().rev() {
+                    // Only show macro locations that are local
+                    // and display them like a span_note
+                    if let Some(def_site) = trace.def_site_span {
+                        if (def_site == COMMAND_LINE_SP) || (def_site == DUMMY_SP) {
+                            continue;
+                        }
+                        // Check to make sure we're not in any <*macros>
+                        if !cm.span_to_filename(def_site).contains("macros>") {
+                            new_labels.push((trace.call_site,
+                                             "in this macro invocation".to_string()));
+                            break;
+                        }
+                    }
+                }
+            }
+            for (label_span, label_text) in new_labels {
+                span.push_span_label(label_span, label_text);
+            }
+            for sp_label in span.span_labels() {
+                if (sp_label.span == COMMAND_LINE_SP) || (sp_label.span == DUMMY_SP) {
+                    continue;
+                }
+                if cm.span_to_filename(sp_label.span.clone()).contains("macros>") {
+                    let v = cm.macro_backtrace(sp_label.span.clone());
+                    if let Some(use_site) = v.last() {
+                        before_after.push((sp_label.span.clone(), use_site.call_site.clone()));
+                    }
+                }
+            }
+            // After we have them, make sure we replace these 'bad' def sites with their use sites
+            for (before, after) in before_after {
+                span.replace(before, after);
+                spans_updated = true;
+            }
+        }
+
+        spans_updated
+    }
+
+    // This does a small "fix" for multispans by looking to see if it can find any that
+    // point directly at <*macros>. Since these are often difficult to read, this
+    // will change the span to point at the use site.
+    fn fix_multispans_in_std_macros(&mut self,
+                                    span: &mut MultiSpan,
+                                    children: &mut Vec<SubDiagnostic>) {
+        let mut spans_updated = self.fix_multispan_in_std_macros(span);
+        for child in &mut children.iter_mut() {
+            spans_updated |= self.fix_multispan_in_std_macros(&mut child.span);
+        }
+        if spans_updated {
+            children.push(SubDiagnostic {
+                level: Level::Note,
+                message: "this error originates in a macro from the standard library".to_string(),
+                span: MultiSpan::new(),
+                render_span: None
+            });
+        }
+    }
+
     fn emit_message_default(&mut self,
                             msp: &MultiSpan,
                             msg: &str,
@@ -528,10 +612,6 @@ impl EmitterWriter {
             }
         }
 
-        if let Some(ref primary_span) = msp.primary_span().as_ref() {
-            self.render_macro_backtrace_old_school(primary_span, &mut buffer)?;
-        }
-
         // final step: take our styled buffer, render it, then output it
         emit_to_destination(&buffer.render(), level, &mut self.dst)?;
 
@@ -578,26 +658,31 @@ impl EmitterWriter {
         }
         Ok(())
     }
-    fn emit_messages_default(&mut self, db: &DiagnosticBuilder) {
-        let max_line_num = self.get_max_line_num(db);
+    fn emit_messages_default(&mut self,
+                             level: &Level,
+                             message: &String,
+                             code: &Option<String>,
+                             span: &MultiSpan,
+                             children: &Vec<SubDiagnostic>) {
+        let max_line_num = self.get_max_line_num(span, children);
         let max_line_num_len = max_line_num.to_string().len();
 
-        match self.emit_message_default(&db.span,
-                                        &db.message,
-                                        &db.code,
-                                        &db.level,
+        match self.emit_message_default(span,
+                                        message,
+                                        code,
+                                        level,
                                         max_line_num_len,
                                         false) {
             Ok(()) => {
-                if !db.children.is_empty() {
+                if !children.is_empty() {
                     let mut buffer = StyledBuffer::new();
                     draw_col_separator_no_space(&mut buffer, 0, max_line_num_len + 1);
-                    match emit_to_destination(&buffer.render(), &db.level, &mut self.dst) {
+                    match emit_to_destination(&buffer.render(), level, &mut self.dst) {
                         Ok(()) => (),
                         Err(e) => panic!("failed to emit error: {}", e)
                     }
                 }
-                for child in &db.children {
+                for child in children {
                     match child.render_span {
                         Some(FullSpan(ref msp)) => {
                             match self.emit_message_default(msp,
@@ -640,29 +725,6 @@ impl EmitterWriter {
             _ => ()
         }
     }
-    fn render_macro_backtrace_old_school(&mut self,
-                                         sp: &Span,
-                                         buffer: &mut StyledBuffer) -> io::Result<()> {
-        if let Some(ref cm) = self.cm {
-            for trace in cm.macro_backtrace(sp.clone()) {
-                let line_offset = buffer.num_lines();
-
-                let mut diag_string =
-                    format!("in this expansion of {}", trace.macro_decl_name);
-                if let Some(def_site_span) = trace.def_site_span {
-                    diag_string.push_str(
-                        &format!(" (defined in {})",
-                            cm.span_to_filename(def_site_span)));
-                }
-                let snippet = cm.span_to_string(trace.call_site);
-                buffer.append(line_offset, &format!("{} ", snippet), Style::NoStyle);
-                buffer.append(line_offset, "note", Style::Level(Level::Note));
-                buffer.append(line_offset, ": ", Style::NoStyle);
-                buffer.append(line_offset, &diag_string, Style::OldSchoolNoteText);
-            }
-        }
-        Ok(())
-    }
 }
 
 fn draw_col_separator(buffer: &mut StyledBuffer, line: usize, col: usize) {

diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
@@ -221,6 +221,25 @@ impl MultiSpan {
         &self.primary_spans
     }
 
+    /// Replaces all occurances of one Span with another. Used to move Spans in areas that don't
+    /// display well (like std macros). Returns true if replacements occurred.
+    pub fn replace(&mut self, before: Span, after: Span) -> bool {
+        let mut replacements_occurred = false;
+        for primary_span in &mut self.primary_spans {
+            if *primary_span == before {
+                *primary_span = after;
+                replacements_occurred = true;
+            }
+        }
+        for span_label in &mut self.span_labels {
+            if span_label.0 == before {
+                span_label.0 = after;
+                replacements_occurred = true;
+            }
+        }
+        replacements_occurred
+    }
+
     /// Returns the strings to highlight. We always ensure that there
     /// is an entry for each of the primary spans -- for each primary
     /// span P, if there is at least one label with span P, we return

diff --git a/src/test/run-fail-fulldeps/qquote.rs b/src/test/run-fail-fulldeps/qquote.rs
@@ -40,8 +40,10 @@ fn main() {
     });
     let cx = &mut cx;
 
+    println!("{}", pprust::expr_to_string(&*quote_expr!(&cx, 23)));
     assert_eq!(pprust::expr_to_string(&*quote_expr!(&cx, 23)), "23");
 
     let expr = quote_expr!(&cx, let x isize = 20;);
+    println!("{}", pprust::expr_to_string(&*expr));
     assert_eq!(pprust::expr_to_string(&*expr), "let x isize = 20;");
 }
diff --git a/src/test/compile-fail/bad-format-args.rs → src/test/ui/codemap_tests/bad-format-args.rs b/src/test/compile-fail/bad-format-args.rs → src/test/ui/codemap_tests/bad-format-args.rs
@@ -8,13 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// error-pattern: requires at least a format string argument
-// error-pattern: in this expansion
-
-// error-pattern: expected token: `,`
-// error-pattern: in this expansion
-// error-pattern: in this expansion
-
 fn main() {
     format!();
     format!("" 1);

diff --git a/src/test/ui/codemap_tests/bad-format-args.stderr b/src/test/ui/codemap_tests/bad-format-args.stderr
@@ -0,0 +1,26 @@
+error: requires at least a format string argument
+  --> $DIR/bad-format-args.rs:12:5
+   |
+12 |     format!();
+   |     ^^^^^^^^^^
+   |
+   = note: this error originates in a macro from the standard library
+
+error: expected token: `,`
+  --> $DIR/bad-format-args.rs:13:5
+   |
+13 |     format!("" 1);
+   |     ^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro from the standard library
+
+error: expected token: `,`
+  --> $DIR/bad-format-args.rs:14:5
+   |
+14 |     format!("", 1 1);
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro from the standard library
+
+error: aborting due to 3 previous errors
+
diff --git a/src/test/compile-fail/issue-28308.rs → src/test/ui/codemap_tests/issue-28308.rs b/src/test/compile-fail/issue-28308.rs → src/test/ui/codemap_tests/issue-28308.rs
@@ -8,10 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// this error is dispayed in `<std macros>`
-// error-pattern:cannot apply unary operator `!` to type `&'static str`
-// error-pattern:in this expansion of assert!
-
 fn main() {
     assert!("foo");
 }
diff --git a/src/test/ui/codemap_tests/issue-28308.stderr b/src/test/ui/codemap_tests/issue-28308.stderr
@@ -0,0 +1,10 @@
+error: cannot apply unary operator `!` to type `&'static str`
+  --> $DIR/issue-28308.rs:12:5
+   |
+12 |     assert!("foo");
+   |     ^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro from the standard library
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/codemap_tests/repair_span_std_macros.rs b/src/test/ui/codemap_tests/repair_span_std_macros.rs
@@ -0,0 +1,13 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let x = vec![];
+}
diff --git a/src/test/ui/codemap_tests/repair_span_std_macros.stderr b/src/test/ui/codemap_tests/repair_span_std_macros.stderr
@@ -0,0 +1,11 @@
+error[E0282]: unable to infer enough type information about `_`
+  --> $DIR/repair_span_std_macros.rs:12:13
+   |
+12 |     let x = vec![];
+   |             ^^^^^^ cannot infer type for `_`
+   |
+   = note: type annotations or generic parameter binding required
+   = note: this error originates in a macro from the standard library
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/cross-crate-macro-backtrace/auxiliary/extern_macro_crate.rs b/src/test/ui/cross-crate-macro-backtrace/auxiliary/extern_macro_crate.rs
@@ -0,0 +1,23 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "dylib"]
+
+pub fn print(_args: std::fmt::Arguments) {}
+
+#[macro_export]
+macro_rules! myprint {
+    ($($arg:tt)*) => (print(format_args!($($arg)*)));
+}
+
+#[macro_export]
+macro_rules! myprintln {
+    ($fmt:expr) => (myprint!(concat!($fmt, "\n")));
+}
diff --git a/src/test/ui/cross-crate-macro-backtrace/main.rs b/src/test/ui/cross-crate-macro-backtrace/main.rs
@@ -0,0 +1,17 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:extern_macro_crate.rs
+#[macro_use(myprintln, myprint)]
+extern crate extern_macro_crate;
+
+fn main() {
+    myprintln!("{}"); //~ ERROR in this macro
+}
diff --git a/src/test/ui/cross-crate-macro-backtrace/main.stderr b/src/test/ui/cross-crate-macro-backtrace/main.stderr
@@ -0,0 +1,10 @@
+error: invalid reference to argument `0` (no arguments given)
+  --> $DIR/main.rs:16:5
+   |
+16 |     myprintln!("{}"); //~ ERROR in this macro
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro from the standard library
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/macros/bad_hello.rs b/src/test/ui/macros/bad_hello.rs
@@ -0,0 +1,13 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    println!(3 + 4);
+}