Skip to content

Update spans that live in std macros to their use sites #35688

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

Closed
Closed
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
86 changes: 72 additions & 14 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

man, this is unfortunate.

self.emit_messages_default(&db.level, &db.message, &db.code, &primary_span, &children);
}
}

Expand Down Expand Up @@ -381,19 +384,69 @@ 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
}

fn fix_multispan_in_std_macros(&mut self, span: &mut MultiSpan) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This could use a comment explaining what it is aiming to do

let mut spans_updated = false;

if let Some(ref cm) = self.cm {
let mut before_after: Vec<(Span, Span)> = vec![];

// First, find all the spans in <std macros> and point instead at their use site
for sp in span.primary_spans() {
if cm.span_to_filename(sp.clone()) == "<std 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()));
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this trailing semicolon is not needed ?

}
}
for sp_label in span.span_labels() {
if cm.span_to_filename(sp_label.span.clone()) == "<std 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
}

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 i in 0..children.len() {
spans_updated |= self.fix_multispan_in_std_macros(&mut children[i].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 @@ -578,26 +631,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
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 of replacements occurred.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/of/if/

pub fn replace(&mut self, before: Span, after: Span) -> bool {
let mut replacements_occurred = false;
for i in 0..self.primary_spans.len() {
if self.primary_spans[i] == before {
self.primary_spans[i] = after;
replacements_occurred = true;
}
}
for i in 0..self.span_labels.len() {
if self.span_labels[i].0 == before {
self.span_labels[i].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
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