Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace macro backtraces with labeled local uses #35702

Merged
merged 3 commits into from
Aug 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 103 additions & 41 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 19 additions & 0 deletions src/libsyntax_pos/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/test/run-fail-fulldeps/qquote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;");
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 26 additions & 0 deletions src/test/ui/codemap_tests/bad-format-args.stderr
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
10 changes: 10 additions & 0 deletions src/test/ui/codemap_tests/issue-28308.stderr
Original file line number Diff line number Diff line change
@@ -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

13 changes: 13 additions & 0 deletions src/test/ui/codemap_tests/repair_span_std_macros.rs
Original file line number Diff line number Diff line change
@@ -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![];
}
11 changes: 11 additions & 0 deletions src/test/ui/codemap_tests/repair_span_std_macros.stderr
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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")));
}
17 changes: 17 additions & 0 deletions src/test/ui/cross-crate-macro-backtrace/main.rs
Original file line number Diff line number Diff line change
@@ -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
}
10 changes: 10 additions & 0 deletions src/test/ui/cross-crate-macro-backtrace/main.stderr
Original file line number Diff line number Diff line change
@@ -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

13 changes: 13 additions & 0 deletions src/test/ui/macros/bad_hello.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Loading