Auto merge of #35702 - jonathandturner:add_backtrace_labels, r=nikoma…

…tsakis

Replace macro backtraces with labeled local uses

This PR (which builds on #35688) follows from the conversations on how best to [handle the macro backtraces](https://internals.rust-lang.org/t/improving-macro-errors/3809).  The feeling there was that there were two different "groups" of users.

The first group, the macro users, rarely (and likely never) want to see the macro backtrace.  This is often more confusing to users as it will be talking about code they didn't write.

The second group, the macro writers, are trying to debug a macro.  They'll want to see something of the backtrace so that they can see where it's going wrong and what the steps were to get there.

For the first group, it seems clear that we don't want to show *any* macro backtrace.  For the second group, we want to show enough to help the macro writer.

This PR uses a heuristic.  It will only show any backtrace steps if they are in the same crate that is being compiled.  This keeps errors in foreign crates from showing to users that didn't need them.

Additionally, in asking around I repeated heard that the middle steps of the backtrace are rarely, if ever, used in practice.  This PR takes and applies this knowledge.  Now, instead of a full backtrace, the user is given the error underline inside of a local macro as well as the use site as a secondary label.  This effectively means seeing the root of the error and the top of the backtrace, eliding the middle steps.

Rather than being the "perfect solution", this PR opts to take an incremental step towards a better experience.  Likely it would help to have additional macro debugging tools, as they could be much more verbose than we'd likely want to use in the error messages themselves.

Some examples follow.

**Example 1**

Before:

<img width="1275" alt="screen shot 2016-08-15 at 4 13 18 pm" src="https://cloud.githubusercontent.com/assets/547158/17682828/3948cea2-6303-11e6-93b4-b567e9d62848.png">

After:

<img width="596" alt="screen shot 2016-08-15 at 4 13 03 pm" src="https://cloud.githubusercontent.com/assets/547158/17682832/3d670d8c-6303-11e6-9bdc-f30a30bf11ac.png">

**Example 2**

Before:

<img width="918" alt="screen shot 2016-08-15 at 4 14 35 pm" src="https://cloud.githubusercontent.com/assets/547158/17682870/722225de-6303-11e6-9175-336a3f7ce308.png">

After:

<img width="483" alt="screen shot 2016-08-15 at 4 15 01 pm" src="https://cloud.githubusercontent.com/assets/547158/17682872/7582cf6c-6303-11e6-9235-f67960f6bd4c.png">

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 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) {

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

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;");
 }

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);

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
+

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");
 }

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
+

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![];
+}

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
+

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")));
+}

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
+}

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
+

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);
+}