From 01d2b4ab6bdb33e8678c43612b81dbbbad32cc93 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 15 Apr 2016 21:23:50 -0400 Subject: port compiletest to use JSON output This uncovered a lot of bugs in compiletest and also some shortcomings of our existing JSON output. We had to add information to the JSON output, such as suggested text and macro backtraces. We also had to fix various bugs in the existing tests. Joint work with jntrnr. --- src/libsyntax/errors/emitter.rs | 47 ++------ src/libsyntax/errors/json.rs | 251 ++++++++++++++++++++++++++-------------- 2 files changed, 176 insertions(+), 122 deletions(-) (limited to 'src/libsyntax/errors') diff --git a/src/libsyntax/errors/emitter.rs b/src/libsyntax/errors/emitter.rs index 61fdc8453d8..0b5234769b2 100644 --- a/src/libsyntax/errors/emitter.rs +++ b/src/libsyntax/errors/emitter.rs @@ -577,46 +577,17 @@ impl EmitterWriter { fn print_macro_backtrace(&mut self, sp: Span) -> io::Result<()> { - let mut last_span = codemap::DUMMY_SP; - let mut span = sp; - - loop { - let span_name_span = self.cm.with_expn_info(span.expn_id, |expn_info| { - expn_info.map(|ei| { - let (pre, post) = match ei.callee.format { - codemap::MacroAttribute(..) => ("#[", "]"), - codemap::MacroBang(..) => ("", "!"), - }; - let macro_decl_name = format!("in this expansion of {}{}{}", - pre, - ei.callee.name(), - post); - let def_site_span = ei.callee.span; - (ei.call_site, macro_decl_name, def_site_span) - }) - }); - let (macro_decl_name, def_site_span) = match span_name_span { - None => break, - Some((sp, macro_decl_name, def_site_span)) => { - span = sp; - (macro_decl_name, def_site_span) - } - }; - - // Don't print recursive invocations - if !span.source_equal(&last_span) { - let mut diag_string = macro_decl_name; - if let Some(def_site_span) = def_site_span { - diag_string.push_str(&format!(" (defined in {})", - self.cm.span_to_filename(def_site_span))); - } - - let snippet = self.cm.span_to_string(span); - print_diagnostic(&mut self.dst, &snippet, Note, &diag_string, None)?; + for trace in self.cm.macro_backtrace(sp) { + 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 {})", + self.cm.span_to_filename(def_site_span))); } - last_span = span; + let snippet = self.cm.span_to_string(sp); + print_diagnostic(&mut self.dst, &snippet, Note, &diag_string, None)?; } - Ok(()) } } diff --git a/src/libsyntax/errors/json.rs b/src/libsyntax/errors/json.rs index f369582bc5c..5a195e9f078 100644 --- a/src/libsyntax/errors/json.rs +++ b/src/libsyntax/errors/json.rs @@ -1,4 +1,4 @@ -// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -20,13 +20,14 @@ // FIXME spec the JSON output properly. -use codemap::{self, Span, MultiSpan, CodeMap}; +use codemap::{self, Span, MacroBacktrace, MultiSpan, CodeMap}; use diagnostics::registry::Registry; use errors::{Level, DiagnosticBuilder, SubDiagnostic, RenderSpan, CodeSuggestion}; use errors::emitter::Emitter; use std::rc::Rc; use std::io::{self, Write}; +use std::vec; use rustc_serialize::json::as_json; @@ -84,8 +85,12 @@ struct Diagnostic<'a> { /// "error: internal compiler error", "error", "warning", "note", "help". level: &'static str, spans: Vec, - /// Assocaited diagnostic messages. + /// Associated diagnostic messages. children: Vec>, + /// The message as rustc would render it. Currently this is only + /// `Some` for "suggestions", but eventually it will include all + /// snippets. + rendered: Option, } #[derive(RustcEncodable)] @@ -101,16 +106,39 @@ struct DiagnosticSpan { column_end: usize, /// Source text from the start of line_start to the end of line_end. text: Vec, + /// If we are suggesting a replacement, this will contain text + /// that should be sliced in atop this span. You may prefer to + /// load the fully rendered version from the parent `Diagnostic`, + /// however. + suggested_replacement: Option, + /// Macro invocations that created the code at this span, if any. + expansion: Option>, } #[derive(RustcEncodable)] struct DiagnosticSpanLine { text: String, + /// 1-based, character offset in self.text. highlight_start: usize, + highlight_end: usize, } +#[derive(RustcEncodable)] +struct DiagnosticSpanMacroExpansion { + /// span where macro was applied to generate this code; note that + /// this may itself derive from a macro (if + /// `span.expansion.is_some()`) + span: DiagnosticSpan, + + /// name of macro that was applied (e.g., "foo!" or "#[derive(Eq)]") + macro_decl_name: String, + + /// span where macro was defined (if known) + def_site_span: Option, +} + #[derive(RustcEncodable)] struct DiagnosticCode { /// The code itself. @@ -132,6 +160,7 @@ impl<'a> Diagnostic<'a> { level: level.to_str(), spans: msp.map_or(vec![], |msp| DiagnosticSpan::from_multispan(msp, je)), children: vec![], + rendered: None, } } @@ -146,6 +175,7 @@ impl<'a> Diagnostic<'a> { level: level.to_str(), spans: DiagnosticSpan::from_render_span(span, je), children: vec![], + rendered: je.render(span), } } @@ -160,6 +190,7 @@ impl<'a> Diagnostic<'a> { children: db.children.iter().map(|c| { Diagnostic::from_sub_diagnostic(c, je) }).collect(), + rendered: None, } } @@ -173,37 +204,93 @@ impl<'a> Diagnostic<'a> { .or_else(|| db.span.as_ref().map(|s| DiagnosticSpan::from_multispan(s, je))) .unwrap_or(vec![]), children: vec![], + rendered: db.render_span.as_ref() + .and_then(|rsp| je.render(rsp)), } } } impl DiagnosticSpan { + fn from_span(span: Span, suggestion: Option<&String>, je: &JsonEmitter) + -> DiagnosticSpan { + // obtain the full backtrace from the `macro_backtrace` + // helper; in some ways, it'd be better to expand the + // backtrace ourselves, but the `macro_backtrace` helper makes + // some decision, such as dropping some frames, and I don't + // want to duplicate that logic here. + let backtrace = je.cm.macro_backtrace(span).into_iter(); + DiagnosticSpan::from_span_and_backtrace(span, suggestion, backtrace, je) + } + + fn from_span_and_backtrace(span: Span, + suggestion: Option<&String>, + mut backtrace: vec::IntoIter, + je: &JsonEmitter) + -> DiagnosticSpan { + let start = je.cm.lookup_char_pos(span.lo); + let end = je.cm.lookup_char_pos(span.hi); + let backtrace_step = + backtrace.next() + .map(|bt| { + let call_site = + Self::from_span_and_backtrace(bt.call_site, + None, + backtrace, + je); + let def_site_span = + bt.def_site_span + .map(|sp| { + Self::from_span_and_backtrace(sp, + None, + vec![].into_iter(), + je) + }); + Box::new(DiagnosticSpanMacroExpansion { + span: call_site, + macro_decl_name: bt.macro_decl_name, + def_site_span: def_site_span, + }) + }); + DiagnosticSpan { + file_name: start.file.name.clone(), + byte_start: span.lo.0, + byte_end: span.hi.0, + line_start: start.line, + line_end: end.line, + column_start: start.col.0 + 1, + column_end: end.col.0 + 1, + text: DiagnosticSpanLine::from_span(span, je), + suggested_replacement: suggestion.cloned(), + expansion: backtrace_step, + } + } + fn from_multispan(msp: &MultiSpan, je: &JsonEmitter) -> Vec { - msp.spans.iter().map(|span| { - let start = je.cm.lookup_char_pos(span.lo); - let end = je.cm.lookup_char_pos(span.hi); - DiagnosticSpan { - file_name: start.file.name.clone(), - byte_start: span.lo.0, - byte_end: span.hi.0, - line_start: start.line, - line_end: end.line, - column_start: start.col.0 + 1, - column_end: end.col.0 + 1, - text: DiagnosticSpanLine::from_span(span, je), - } - }).collect() + msp.spans.iter().map(|&span| Self::from_span(span, None, je)).collect() + } + + fn from_suggestion(suggestion: &CodeSuggestion, je: &JsonEmitter) + -> Vec { + assert_eq!(suggestion.msp.spans.len(), suggestion.substitutes.len()); + suggestion.msp.spans.iter() + .zip(&suggestion.substitutes) + .map(|(&span, suggestion)| { + DiagnosticSpan::from_span(span, Some(suggestion), je) + }) + .collect() } fn from_render_span(rsp: &RenderSpan, je: &JsonEmitter) -> Vec { match *rsp { - RenderSpan::FullSpan(ref msp) | - // FIXME(#30701) handle Suggestion properly - RenderSpan::Suggestion(CodeSuggestion { ref msp, .. }) => { + RenderSpan::FileLine(ref msp) | + RenderSpan::FullSpan(ref msp) => { DiagnosticSpan::from_multispan(msp, je) } + RenderSpan::Suggestion(ref suggestion) => { + DiagnosticSpan::from_suggestion(suggestion, je) + } RenderSpan::EndSpan(ref msp) => { - msp.spans.iter().map(|span| { + msp.spans.iter().map(|&span| { let end = je.cm.lookup_char_pos(span.hi); DiagnosticSpan { file_name: end.file.name.clone(), @@ -214,37 +301,11 @@ impl DiagnosticSpan { column_start: end.col.0 + 1, column_end: end.col.0 + 1, text: DiagnosticSpanLine::from_span_end(span, je), + suggested_replacement: None, + expansion: None, } }).collect() } - RenderSpan::FileLine(ref msp) => { - msp.spans.iter().map(|span| { - let start = je.cm.lookup_char_pos(span.lo); - let end = je.cm.lookup_char_pos(span.hi); - DiagnosticSpan { - file_name: start.file.name.clone(), - byte_start: span.lo.0, - byte_end: span.hi.0, - line_start: start.line, - line_end: end.line, - column_start: 0, - column_end: 0, - text: DiagnosticSpanLine::from_span(span, je), - } - }).collect() - } - } - } -} - -macro_rules! get_lines_for_span { - ($span: ident, $je: ident) => { - match $je.cm.span_to_lines(*$span) { - Ok(lines) => lines, - Err(_) => { - debug!("unprintable span"); - return Vec::new(); - } } } } @@ -265,45 +326,49 @@ impl DiagnosticSpanLine { /// Create a list of DiagnosticSpanLines from span - each line with any part /// of `span` gets a DiagnosticSpanLine, with the highlight indicating the /// `span` within the line. - fn from_span(span: &Span, je: &JsonEmitter) -> Vec { - let lines = get_lines_for_span!(span, je); - - let mut result = Vec::new(); - let fm = &*lines.file; - - for line in &lines.lines { - result.push(DiagnosticSpanLine::line_from_filemap(fm, - line.line_index, - line.start_col.0 + 1, - line.end_col.0 + 1)); - } - - result + fn from_span(span: Span, je: &JsonEmitter) -> Vec { + je.cm.span_to_lines(span) + .map(|lines| { + let fm = &*lines.file; + lines.lines + .iter() + .map(|line| { + DiagnosticSpanLine::line_from_filemap(fm, + line.line_index, + line.start_col.0 + 1, + line.end_col.0 + 1) + }) + .collect() + }) + .unwrap_or(vec![]) } /// Create a list of DiagnosticSpanLines from span - the result covers all /// of `span`, but the highlight is zero-length and at the end of `span`. - fn from_span_end(span: &Span, je: &JsonEmitter) -> Vec { - let lines = get_lines_for_span!(span, je); - - let mut result = Vec::new(); - let fm = &*lines.file; - - for (i, line) in lines.lines.iter().enumerate() { - // Invariant - CodeMap::span_to_lines will not return extra context - // lines - the last line returned is the last line of `span`. - let highlight = if i == lines.lines.len() - 1 { - (line.end_col.0 + 1, line.end_col.0 + 1) - } else { - (0, 0) - }; - result.push(DiagnosticSpanLine::line_from_filemap(fm, - line.line_index, - highlight.0, - highlight.1)); - } - - result + fn from_span_end(span: Span, je: &JsonEmitter) -> Vec { + je.cm.span_to_lines(span) + .map(|lines| { + let fm = &*lines.file; + lines.lines.iter() + .enumerate() + .map(|(i, line)| { + // Invariant - CodeMap::span_to_lines + // will not return extra context lines + // - the last line returned is the last + // line of `span`. + let highlight = if i == lines.lines.len() - 1 { + (line.end_col.0 + 1, line.end_col.0 + 1) + } else { + (0, 0) + }; + DiagnosticSpanLine::line_from_filemap(fm, + line.line_index, + highlight.0, + highlight.1) + }) + .collect() + }) + .unwrap_or(vec![]) } } @@ -322,3 +387,21 @@ impl DiagnosticCode { }) } } + +impl JsonEmitter { + fn render(&self, render_span: &RenderSpan) -> Option { + match *render_span { + RenderSpan::FileLine(_) | + RenderSpan::FullSpan(_) => { + None + } + RenderSpan::Suggestion(ref suggestion) => { + Some(suggestion.splice_lines(&self.cm)) + } + RenderSpan::EndSpan(_) => { + None + } + } + } +} + -- cgit 1.4.1-3-g733a5 From 28a3c881576ee09fe8ab9dfff4a15ffb89e1c149 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 18 Apr 2016 05:04:46 -0400 Subject: pacify the merciless acrichto (somewhat) Also add a comment or two to pacify the merciless self-critic, who hates a closure without a comment. --- src/compiletest/json.rs | 23 ++++++++++++++++------- src/libsyntax/errors/json.rs | 14 ++++++-------- 2 files changed, 22 insertions(+), 15 deletions(-) (limited to 'src/libsyntax/errors') diff --git a/src/compiletest/json.rs b/src/compiletest/json.rs index 69274689e5c..bbe038509e3 100644 --- a/src/compiletest/json.rs +++ b/src/compiletest/json.rs @@ -69,8 +69,7 @@ fn parse_line(file_name: &str, line: &str) -> Vec { expected_errors } Err(error) => { - println!("failed to decode compiler output as json: `{}`", error); - panic!("failed to decode compiler output as json"); + panic!("failed to decode compiler output as json: `{}`", error); } } } else { @@ -82,10 +81,19 @@ fn push_expected_errors(expected_errors: &mut Vec, diagnostic: &Diagnostic, file_name: &str) { // We only consider messages pertaining to the current file. - let matching_spans = - || diagnostic.spans.iter().filter(|span| span.file_name == file_name); - let with_code = - |span: &DiagnosticSpan, text: &str| match diagnostic.code { + let matching_spans = || { + diagnostic.spans.iter().filter(|span| span.file_name == file_name) + }; + + // We break the output into multiple lines, and then append the + // [E123] to every line in the output. This may be overkill. The + // intention was to match existing tests that do things like "//| + // found `i32` [E123]" and expect to match that somewhere, and yet + // also ensure that `//~ ERROR E123` *always* works. The + // assumption is that these multi-line error messages are on their + // way out anyhow. + let with_code = |span: &DiagnosticSpan, text: &str| { + match diagnostic.code { Some(ref code) => // FIXME(#33000) -- it'd be better to use a dedicated // UI harness than to include the line/col number like @@ -105,7 +113,8 @@ fn push_expected_errors(expected_errors: &mut Vec, span.line_start, span.column_start, span.line_end, span.column_end, text), - }; + } + }; // Convert multi-line messages into multiple expected // errors. We expect to replace these with something diff --git a/src/libsyntax/errors/json.rs b/src/libsyntax/errors/json.rs index 5a195e9f078..821617bfe89 100644 --- a/src/libsyntax/errors/json.rs +++ b/src/libsyntax/errors/json.rs @@ -237,14 +237,12 @@ impl DiagnosticSpan { None, backtrace, je); - let def_site_span = - bt.def_site_span - .map(|sp| { - Self::from_span_and_backtrace(sp, - None, - vec![].into_iter(), - je) - }); + let def_site_span = bt.def_site_span.map(|sp| { + Self::from_span_and_backtrace(sp, + None, + vec![].into_iter(), + je) + }); Box::new(DiagnosticSpanMacroExpansion { span: call_site, macro_decl_name: bt.macro_decl_name, -- cgit 1.4.1-3-g733a5