From 0f46e4f1f23368f4615a9847671e3a91b2ebaf18 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Fri, 3 Apr 2015 01:27:04 -0700 Subject: [PATCH 1/5] Propagate macro backtraces more often, improve formatting diagnostics * In noop_fold_expr, call new_span in these cases: - ExprMethodCall's identifier - ExprField's identifier - ExprTupField's integer Calling new_span for ExprMethodCall's identifier is necessary to print an acceptable diagnostic for write!(&2, ""). We see this error: :2:20: 2:66 error: type `&mut _` does not implement any method in scope named `write_fmt` :2 ( & mut * $ dst ) . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ With this change, we also see a macro expansion backtrace leading to the write!(&2, "") call site. * After fully expanding a macro, we replace the expansion expression's span with the original span. Call fld.new_span to add a backtrace to this span. (Note that I'm call new_span after bt.pop(), so the macro just expanded isn't on the backtrace.) The motivating example for this change is println!("{}"). The format string literal is concat!($fmt, "arg") and is inside the libstd macro. We need to see the backtrace to find the println! call site. * Add a backtrace to the format_args! format expression span. Addresses #23459 --- src/libsyntax/ext/expand.rs | 2 +- src/libsyntax/ext/format.rs | 8 ++++++-- src/libsyntax/fold.rs | 8 +++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 5f4ec01791c..b65798b8a49 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -55,7 +55,7 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { fully_expanded.map(|e| ast::Expr { id: ast::DUMMY_NODE_ID, node: e.node, - span: span, + span: fld.new_span(span), }) } diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 1d99a475b32..513bbf6c77b 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -17,6 +17,7 @@ use ext::base::*; use ext::base; use ext::build::AstBuilder; use fmt_macros as parse; +use fold::Folder; use parse::token::special_idents; use parse::token; use ptr::P; @@ -649,6 +650,10 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, sp: Span, names: HashMap>) -> P { let arg_types: Vec<_> = (0..args.len()).map(|_| None).collect(); + // Expand the format literal so that efmt.span will have a backtrace. This + // is essential for locating a bug when the format literal is generated in + // a macro. (e.g. println!("{}"), which uses concat!($fmt, "\n")). + let efmt = ecx.expander().fold_expr(efmt); let mut cx = Context { ecx: ecx, args: args, @@ -663,9 +668,8 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, sp: Span, pieces: Vec::new(), str_pieces: Vec::new(), all_pieces_simple: true, - fmtsp: sp, + fmtsp: efmt.span, }; - cx.fmtsp = efmt.span; let fmt = match expr_to_string(cx.ecx, efmt, "format argument must be a string literal.") { diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index d4451cc7b71..d7033ce7e48 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1176,7 +1176,7 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> } ExprMethodCall(i, tps, args) => { ExprMethodCall( - respan(i.span, folder.fold_ident(i.node)), + respan(folder.new_span(i.span), folder.fold_ident(i.node)), tps.move_map(|x| folder.fold_ty(x)), args.move_map(|x| folder.fold_expr(x))) } @@ -1246,11 +1246,13 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> } ExprField(el, ident) => { ExprField(folder.fold_expr(el), - respan(ident.span, folder.fold_ident(ident.node))) + respan(folder.new_span(ident.span), + folder.fold_ident(ident.node))) } ExprTupField(el, ident) => { ExprTupField(folder.fold_expr(el), - respan(ident.span, folder.fold_usize(ident.node))) + respan(folder.new_span(ident.span), + folder.fold_usize(ident.node))) } ExprIndex(el, er) => { ExprIndex(folder.fold_expr(el), folder.fold_expr(er)) From a893c646d0ba9b58fb6167dca6fbbc567b479c95 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Fri, 3 Apr 2015 20:22:36 -0700 Subject: [PATCH 2/5] Expand internal-unstable to handle named field accesses and method calls. --- src/test/auxiliary/internal_unstable.rs | 33 +++++++++++++++++++ .../compile-fail/internal-unstable-noallow.rs | 6 ++++ src/test/compile-fail/internal-unstable.rs | 2 ++ 3 files changed, 41 insertions(+) diff --git a/src/test/auxiliary/internal_unstable.rs b/src/test/auxiliary/internal_unstable.rs index 3d59b8e9009..7f682d5d8d1 100644 --- a/src/test/auxiliary/internal_unstable.rs +++ b/src/test/auxiliary/internal_unstable.rs @@ -22,6 +22,17 @@ pub struct Foo { pub x: u8 } +impl Foo { + #[unstable(feature = "method")] + pub fn method(&self) {} +} + +#[stable(feature = "stable", since = "1.0.0")] +pub struct Bar { + #[unstable(feature = "struct2_field")] + pub x: u8 +} + #[allow_internal_unstable] #[macro_export] macro_rules! call_unstable_allow { @@ -36,6 +47,18 @@ macro_rules! construct_unstable_allow { } } +#[allow_internal_unstable] +#[macro_export] +macro_rules! call_method_allow { + ($e: expr) => { $e.method() } +} + +#[allow_internal_unstable] +#[macro_export] +macro_rules! access_field_allow { + ($e: expr) => { $e.x } +} + #[allow_internal_unstable] #[macro_export] macro_rules! pass_through_allow { @@ -54,6 +77,16 @@ macro_rules! construct_unstable_noallow { } } +#[macro_export] +macro_rules! call_method_noallow { + ($e: expr) => { $e.method() } +} + +#[macro_export] +macro_rules! access_field_noallow { + ($e: expr) => { $e.x } +} + #[macro_export] macro_rules! pass_through_noallow { ($e: expr) => { $e } diff --git a/src/test/compile-fail/internal-unstable-noallow.rs b/src/test/compile-fail/internal-unstable-noallow.rs index 2b48d47e940..2e42e9d3b01 100644 --- a/src/test/compile-fail/internal-unstable-noallow.rs +++ b/src/test/compile-fail/internal-unstable-noallow.rs @@ -16,6 +16,8 @@ // aux-build:internal_unstable.rs // error-pattern:use of unstable library feature 'function' // error-pattern:use of unstable library feature 'struct_field' +// error-pattern:use of unstable library feature 'method' +// error-pattern:use of unstable library feature 'struct2_field' #[macro_use] extern crate internal_unstable; @@ -24,4 +26,8 @@ fn main() { call_unstable_noallow!(); construct_unstable_noallow!(0); + + |x: internal_unstable::Foo| { call_method_noallow!(x) }; + + |x: internal_unstable::Bar| { access_field_noallow!(x) }; } diff --git a/src/test/compile-fail/internal-unstable.rs b/src/test/compile-fail/internal-unstable.rs index accc898b8a8..e01259f0deb 100755 --- a/src/test/compile-fail/internal-unstable.rs +++ b/src/test/compile-fail/internal-unstable.rs @@ -36,6 +36,8 @@ fn main() { // ok, the instability is contained. call_unstable_allow!(); construct_unstable_allow!(0); + |x: internal_unstable::Foo| { call_method_allow!(x) }; + |x: internal_unstable::Bar| { access_field_allow!(x) }; // bad. pass_through_allow!(internal_unstable::unstable()); //~ ERROR use of unstable From 5a8f102bf6065901a6115c02dab6fa29b71b2db6 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Fri, 3 Apr 2015 19:58:41 -0700 Subject: [PATCH 3/5] Add compile-file/macro-backtrace-{invalid-internals,nested,println} tests --- .../macro-backtrace-invalid-internals.rs | 57 +++++++++++++++++++ .../compile-fail/macro-backtrace-nested.rs | 29 ++++++++++ .../compile-fail/macro-backtrace-println.rs | 29 ++++++++++ 3 files changed, 115 insertions(+) create mode 100644 src/test/compile-fail/macro-backtrace-invalid-internals.rs create mode 100644 src/test/compile-fail/macro-backtrace-nested.rs create mode 100644 src/test/compile-fail/macro-backtrace-println.rs diff --git a/src/test/compile-fail/macro-backtrace-invalid-internals.rs b/src/test/compile-fail/macro-backtrace-invalid-internals.rs new file mode 100644 index 00000000000..df906d72356 --- /dev/null +++ b/src/test/compile-fail/macro-backtrace-invalid-internals.rs @@ -0,0 +1,57 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Macros in statement vs expression position handle backtraces differently. + +macro_rules! fake_method_stmt { //~ NOTE in expansion of + () => { + 1.fake() //~ ERROR does not implement any method + } +} + +macro_rules! fake_field_stmt { //~ NOTE in expansion of + () => { + 1.fake //~ ERROR no field with that name + } +} + +macro_rules! fake_anon_field_stmt { //~ NOTE in expansion of + () => { + (1).0 //~ ERROR type was not a tuple + } +} + +macro_rules! fake_method_expr { //~ NOTE in expansion of + () => { + 1.fake() //~ ERROR does not implement any method + } +} + +macro_rules! fake_field_expr { + () => { + 1.fake + } +} + +macro_rules! fake_anon_field_expr { + () => { + (1).0 + } +} + +fn main() { + fake_method_stmt!(); //~ NOTE expansion site + fake_field_stmt!(); //~ NOTE expansion site + fake_anon_field_stmt!(); //~ NOTE expansion site + + let _ = fake_method_expr!(); //~ NOTE expansion site + let _ = fake_field_expr!(); //~ ERROR no field with that name + let _ = fake_anon_field_expr!(); //~ ERROR type was not a tuple +} diff --git a/src/test/compile-fail/macro-backtrace-nested.rs b/src/test/compile-fail/macro-backtrace-nested.rs new file mode 100644 index 00000000000..7c1dc1a468c --- /dev/null +++ b/src/test/compile-fail/macro-backtrace-nested.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// In expression position, but not statement position, when we expand a macro, +// we replace the span of the expanded expression with that of the call site. + +macro_rules! nested_expr { + () => (fake) +} + +macro_rules! call_nested_expr { + () => (nested_expr!()) +} + +macro_rules! call_nested_expr_sum { //~ NOTE in expansion of + () => { 1 + nested_expr!(); } //~ ERROR unresolved name +} + +fn main() { + 1 + call_nested_expr!(); //~ ERROR unresolved name + call_nested_expr_sum!(); //~ NOTE expansion site +} diff --git a/src/test/compile-fail/macro-backtrace-println.rs b/src/test/compile-fail/macro-backtrace-println.rs new file mode 100644 index 00000000000..0c66bbfcf04 --- /dev/null +++ b/src/test/compile-fail/macro-backtrace-println.rs @@ -0,0 +1,29 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// The `format_args!` syntax extension issues errors before code expansion +// has completed, but we still need a backtrace. + +// This test includes stripped-down versions of `print!` and `println!`, +// because we can't otherwise verify the lines of the backtrace. + +fn print(_args: std::fmt::Arguments) {} + +macro_rules! myprint { //~ NOTE in expansion of + ($($arg:tt)*) => (print(format_args!($($arg)*))); +} + +macro_rules! myprintln { //~ NOTE in expansion of + ($fmt:expr) => (myprint!(concat!($fmt, "\n"))); //~ ERROR invalid reference to argument `0` +} + +fn main() { + myprintln!("{}"); //~ NOTE expansion site +} From fab3295cba64a8e4245374c5eb8f2478029d13db Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Sun, 5 Apr 2015 18:07:11 -0700 Subject: [PATCH 4/5] Suppress the macro backtrace for `fileline_note` and `fileline_help`. --- src/libsyntax/codemap.rs | 2 +- src/libsyntax/diagnostic.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 7635c8eadc2..5bbf79d0477 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -871,7 +871,7 @@ impl CodeMap { F: FnOnce(Option<&ExpnInfo>) -> T, { match id { - NO_EXPANSION => f(None), + NO_EXPANSION | COMMAND_LINE_EXPN => f(None), ExpnId(i) => f(Some(&(*self.expansions.borrow())[i as usize])) } } diff --git a/src/libsyntax/diagnostic.rs b/src/libsyntax/diagnostic.rs index ed7bdcd898e..f3715d765e3 100644 --- a/src/libsyntax/diagnostic.rs +++ b/src/libsyntax/diagnostic.rs @@ -465,22 +465,21 @@ fn emit(dst: &mut EmitterWriter, cm: &codemap::CodeMap, rsp: RenderSpan, match rsp { FullSpan(_) => { try!(highlight_lines(dst, cm, sp, lvl, cm.span_to_lines(sp))); + try!(print_macro_backtrace(dst, cm, sp)); } EndSpan(_) => { try!(end_highlight_lines(dst, cm, sp, lvl, cm.span_to_lines(sp))); + try!(print_macro_backtrace(dst, cm, sp)); } Suggestion(_, ref suggestion) => { try!(highlight_suggestion(dst, cm, sp, suggestion)); + try!(print_macro_backtrace(dst, cm, sp)); } FileLine(..) => { // no source text in this case! } } - if sp != COMMAND_LINE_SP { - try!(print_macro_backtrace(dst, cm, sp)); - } - match code { Some(code) => match dst.registry.as_ref().and_then(|registry| registry.find_description(code)) { From ddbdf51f394226bcae162ed2d5348126b32e7dbd Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Thu, 9 Apr 2015 22:42:59 -0700 Subject: [PATCH 5/5] Remove the vestigial ExtCtxt::print_backtrace function. It was added in 2011-08-05 and reduced to a no-op ten days later. --- src/libsyntax/ext/base.rs | 10 ---------- src/libsyntax/ext/log_syntax.rs | 2 -- 2 files changed, 12 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 346fb3580e1..9994fad3e31 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -605,7 +605,6 @@ impl<'a> ExtCtxt<'a> { None => self.bug("missing top span") }) } - pub fn print_backtrace(&self) { } pub fn backtrace(&self) -> ExpnId { self.backtrace } pub fn original_span(&self) -> Span { let mut expn_id = self.backtrace; @@ -700,7 +699,6 @@ impl<'a> ExtCtxt<'a> { /// substitute; we never hit resolve/type-checking so the dummy /// value doesn't have to match anything) pub fn span_fatal(&self, sp: Span, msg: &str) -> ! { - self.print_backtrace(); panic!(self.parse_sess.span_diagnostic.span_fatal(sp, msg)); } @@ -710,35 +708,27 @@ impl<'a> ExtCtxt<'a> { /// Compilation will be stopped in the near future (at the end of /// the macro expansion phase). pub fn span_err(&self, sp: Span, msg: &str) { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_err(sp, msg); } pub fn span_warn(&self, sp: Span, msg: &str) { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_warn(sp, msg); } pub fn span_unimpl(&self, sp: Span, msg: &str) -> ! { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_unimpl(sp, msg); } pub fn span_bug(&self, sp: Span, msg: &str) -> ! { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_bug(sp, msg); } pub fn span_note(&self, sp: Span, msg: &str) { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_note(sp, msg); } pub fn span_help(&self, sp: Span, msg: &str) { - self.print_backtrace(); self.parse_sess.span_diagnostic.span_help(sp, msg); } pub fn fileline_help(&self, sp: Span, msg: &str) { - self.print_backtrace(); self.parse_sess.span_diagnostic.fileline_help(sp, msg); } pub fn bug(&self, msg: &str) -> ! { - self.print_backtrace(); self.parse_sess.span_diagnostic.handler().bug(msg); } pub fn trace_macros(&self) -> bool { diff --git a/src/libsyntax/ext/log_syntax.rs b/src/libsyntax/ext/log_syntax.rs index 8173dd93f74..9869108952c 100644 --- a/src/libsyntax/ext/log_syntax.rs +++ b/src/libsyntax/ext/log_syntax.rs @@ -26,8 +26,6 @@ pub fn expand_syntax_ext<'cx>(cx: &'cx mut base::ExtCtxt, return base::DummyResult::any(sp); } - cx.print_backtrace(); - println!("{}", print::pprust::tts_to_string(tts)); // any so that `log_syntax` can be invoked as an expression and item.