From 787c458eeb20e447989124e2900e02e8e03b1c51 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 20 Jan 2020 22:22:36 +0200 Subject: [PATCH 1/4] Don't use ExpnKind::descr to get the name of a bang macro. --- src/librustc_lint/internal.rs | 6 ++++-- src/librustc_save_analysis/lib.rs | 21 ++++++++++++++------- src/librustc_span/hygiene.rs | 6 ++++-- src/tools/clippy | 2 +- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/librustc_lint/internal.rs b/src/librustc_lint/internal.rs index 91aeccbb5e3..8480c85075d 100644 --- a/src/librustc_lint/internal.rs +++ b/src/librustc_lint/internal.rs @@ -6,6 +6,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_hir::{GenericArg, HirId, MutTy, Mutability, Path, PathSegment, QPath, Ty, TyKind}; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; +use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::symbol::{sym, Symbol}; use syntax::ast::{Ident, Item, ItemKind}; @@ -226,8 +227,9 @@ impl EarlyLintPass for LintPassImpl { if last.ident.name == sym::LintPass { let expn_data = lint_pass.path.span.ctxt().outer_expn_data(); let call_site = expn_data.call_site; - if expn_data.kind.descr() != sym::impl_lint_pass - && call_site.ctxt().outer_expn_data().kind.descr() != sym::declare_lint_pass + if expn_data.kind != ExpnKind::Macro(MacroKind::Bang, sym::impl_lint_pass) + && call_site.ctxt().outer_expn_data().kind + != ExpnKind::Macro(MacroKind::Bang, sym::declare_lint_pass) { cx.struct_span_lint( LINT_PASS_IMPL_WITHOUT_MACRO, diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 537fe198a0c..f44ce6f4eac 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -776,12 +776,19 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { let callsite_span = self.span_from_span(callsite); let callee = span.source_callee()?; - // Ignore attribute macros, their spans are usually mangled - if let ExpnKind::Macro(MacroKind::Attr, _) | ExpnKind::Macro(MacroKind::Derive, _) = - callee.kind - { - return None; - } + let mac_name = match callee.kind { + ExpnKind::Macro(mac_kind, name) => match mac_kind { + MacroKind::Bang => name, + + // Ignore attribute macros, their spans are usually mangled + // FIXME(eddyb) is this really the case anymore? + MacroKind::Attr | MacroKind::Derive => return None, + }, + + // These are not macros. + // FIXME(eddyb) maybe there is a way to handle them usefully? + ExpnKind::Root | ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => return None, + }; // If the callee is an imported macro from an external crate, need to get // the source span and name from the session, as their spans are localized @@ -799,7 +806,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { let callee_span = self.span_from_span(callee.def_site); Some(MacroRef { span: callsite_span, - qualname: callee.kind.descr().to_string(), // FIXME: generate the real qualname + qualname: mac_name.to_string(), // FIXME: generate the real qualname callee_span, }) } diff --git a/src/librustc_span/hygiene.rs b/src/librustc_span/hygiene.rs index fd1f07c743b..366201d66c4 100644 --- a/src/librustc_span/hygiene.rs +++ b/src/librustc_span/hygiene.rs @@ -140,7 +140,9 @@ impl ExpnId { loop { let expn_data = self.expn_data(); // Stop going up the backtrace once include! is encountered - if expn_data.is_root() || expn_data.kind.descr() == sym::include { + if expn_data.is_root() + || expn_data.kind == ExpnKind::Macro(MacroKind::Bang, sym::include) + { break; } self = expn_data.call_site.ctxt().outer_expn(); @@ -717,7 +719,7 @@ impl ExpnData { } /// Expansion kind. -#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable_Generic)] +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable, HashStable_Generic)] pub enum ExpnKind { /// No expansion, aka root expansion. Only `ExpnId::root()` has this kind. Root, diff --git a/src/tools/clippy b/src/tools/clippy index 3e74853d1f9..fa046d2e7f1 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit 3e74853d1f9893cf2a47f28b658711d8f9f97b6b +Subproject commit fa046d2e7f14cda09d14230cc8c772e1565e0757 From a7b0aa0675f6e81bdb62e614c020a6862381c98a Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 21 Jan 2020 01:02:01 +0200 Subject: [PATCH 2/4] rustc_span: move pretty syntax from macro_backtrace to ExpnKind::descr. --- src/librustc_expand/expand.rs | 5 +---- src/librustc_span/hygiene.rs | 14 +++++++++----- src/librustc_span/lib.rs | 12 +----------- .../ui/did_you_mean/recursion_limit_macro.stderr | 2 +- src/test/ui/infinite/infinite-macro-expansion.rs | 2 +- .../ui/infinite/infinite-macro-expansion.stderr | 2 +- src/test/ui/issues/issue-16098.rs | 2 +- src/test/ui/issues/issue-16098.stderr | 2 +- src/test/ui/macros/trace_faulty_macros.stderr | 2 +- src/tools/cargo | 2 +- 10 files changed, 18 insertions(+), 27 deletions(-) diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index 3254d0c913d..f915f44c17a 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -596,10 +596,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { let suggested_limit = self.cx.ecfg.recursion_limit * 2; let mut err = self.cx.struct_span_err( expn_data.call_site, - &format!( - "recursion limit reached while expanding the macro `{}`", - expn_data.kind.descr() - ), + &format!("recursion limit reached while expanding `{}`", expn_data.kind.descr()), ); err.help(&format!( "consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate", diff --git a/src/librustc_span/hygiene.rs b/src/librustc_span/hygiene.rs index 366201d66c4..a368a881674 100644 --- a/src/librustc_span/hygiene.rs +++ b/src/librustc_span/hygiene.rs @@ -732,12 +732,16 @@ pub enum ExpnKind { } impl ExpnKind { - pub fn descr(&self) -> Symbol { + pub fn descr(&self) -> String { match *self { - ExpnKind::Root => kw::PathRoot, - ExpnKind::Macro(_, descr) => descr, - ExpnKind::AstPass(kind) => Symbol::intern(kind.descr()), - ExpnKind::Desugaring(kind) => Symbol::intern(kind.descr()), + ExpnKind::Root => kw::PathRoot.to_string(), + ExpnKind::Macro(macro_kind, name) => match macro_kind { + MacroKind::Bang => format!("{}!", name), + MacroKind::Attr => format!("#[{}]", name), + MacroKind::Derive => format!("#[derive({})]", name), + }, + ExpnKind::AstPass(kind) => kind.descr().to_string(), + ExpnKind::Desugaring(kind) => format!("desugaring of {}", kind.descr()), } } } diff --git a/src/librustc_span/lib.rs b/src/librustc_span/lib.rs index 5779d17e3e5..764312021ef 100644 --- a/src/librustc_span/lib.rs +++ b/src/librustc_span/lib.rs @@ -455,19 +455,9 @@ impl Span { } // Don't print recursive invocations. if !expn_data.call_site.source_equal(&prev_span) { - let (pre, post) = match expn_data.kind { - ExpnKind::Root => break, - ExpnKind::Desugaring(..) => ("desugaring of ", ""), - ExpnKind::AstPass(..) => ("", ""), - ExpnKind::Macro(macro_kind, _) => match macro_kind { - MacroKind::Bang => ("", "!"), - MacroKind::Attr => ("#[", "]"), - MacroKind::Derive => ("#[derive(", ")]"), - }, - }; result.push(MacroBacktrace { call_site: expn_data.call_site, - macro_decl_name: format!("{}{}{}", pre, expn_data.kind.descr(), post), + macro_decl_name: expn_data.kind.descr(), def_site_span: expn_data.def_site, }); } diff --git a/src/test/ui/did_you_mean/recursion_limit_macro.stderr b/src/test/ui/did_you_mean/recursion_limit_macro.stderr index 6640ced5c9e..1cc59051605 100644 --- a/src/test/ui/did_you_mean/recursion_limit_macro.stderr +++ b/src/test/ui/did_you_mean/recursion_limit_macro.stderr @@ -1,4 +1,4 @@ -error: recursion limit reached while expanding the macro `recurse` +error: recursion limit reached while expanding `recurse!` --> $DIR/recursion_limit_macro.rs:10:31 | LL | ($t:tt $($tail:tt)*) => { recurse!($($tail)*) }; diff --git a/src/test/ui/infinite/infinite-macro-expansion.rs b/src/test/ui/infinite/infinite-macro-expansion.rs index 968d8360bb0..6ea0bc73dc0 100644 --- a/src/test/ui/infinite/infinite-macro-expansion.rs +++ b/src/test/ui/infinite/infinite-macro-expansion.rs @@ -1,5 +1,5 @@ macro_rules! recursive { - () => (recursive!()) //~ ERROR recursion limit reached while expanding the macro `recursive` + () => (recursive!()) //~ ERROR recursion limit reached while expanding `recursive!` } fn main() { diff --git a/src/test/ui/infinite/infinite-macro-expansion.stderr b/src/test/ui/infinite/infinite-macro-expansion.stderr index 0c0c6596760..159312e5c1b 100644 --- a/src/test/ui/infinite/infinite-macro-expansion.stderr +++ b/src/test/ui/infinite/infinite-macro-expansion.stderr @@ -1,4 +1,4 @@ -error: recursion limit reached while expanding the macro `recursive` +error: recursion limit reached while expanding `recursive!` --> $DIR/infinite-macro-expansion.rs:2:12 | LL | () => (recursive!()) diff --git a/src/test/ui/issues/issue-16098.rs b/src/test/ui/issues/issue-16098.rs index a1131f80e90..00acc20fc9e 100644 --- a/src/test/ui/issues/issue-16098.rs +++ b/src/test/ui/issues/issue-16098.rs @@ -4,7 +4,7 @@ macro_rules! prob1 { }; ($n:expr) => { if ($n % 3 == 0) || ($n % 5 == 0) { - $n + prob1!($n - 1); //~ ERROR recursion limit reached while expanding the macro `prob1` + $n + prob1!($n - 1); //~ ERROR recursion limit reached while expanding `prob1!` } else { prob1!($n - 1); } diff --git a/src/test/ui/issues/issue-16098.stderr b/src/test/ui/issues/issue-16098.stderr index f890baf8eba..2b9657d4628 100644 --- a/src/test/ui/issues/issue-16098.stderr +++ b/src/test/ui/issues/issue-16098.stderr @@ -1,4 +1,4 @@ -error: recursion limit reached while expanding the macro `prob1` +error: recursion limit reached while expanding `prob1!` --> $DIR/issue-16098.rs:7:18 | LL | $n + prob1!($n - 1); diff --git a/src/test/ui/macros/trace_faulty_macros.stderr b/src/test/ui/macros/trace_faulty_macros.stderr index f06e6581ff7..4e86daffb61 100644 --- a/src/test/ui/macros/trace_faulty_macros.stderr +++ b/src/test/ui/macros/trace_faulty_macros.stderr @@ -20,7 +20,7 @@ LL | my_faulty_macro!(); = note: to `my_faulty_macro ! (bcd) ;` = note: expanding `my_faulty_macro! { bcd }` -error: recursion limit reached while expanding the macro `my_recursive_macro` +error: recursion limit reached while expanding `my_recursive_macro!` --> $DIR/trace_faulty_macros.rs:22:9 | LL | my_recursive_macro!(); diff --git a/src/tools/cargo b/src/tools/cargo index f6449ba236d..b68b0978ab8 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit f6449ba236db31995255ac5e4cad4ab88296a7c6 +Subproject commit b68b0978ab8012f871c80736fb910d14b89c4498 From 75284f8cbdfa17046156528dc3aa5303f8752f97 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 21 Jan 2020 01:27:14 +0200 Subject: [PATCH 3/4] rustc_span: replace MacroBacktrace with ExpnData. --- src/librustc_errors/emitter.rs | 12 ++++++------ src/librustc_errors/json.rs | 9 +++++---- src/librustc_span/lib.rs | 20 ++------------------ 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 7218730538a..49c8be28292 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -21,6 +21,7 @@ use crate::{ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; +use rustc_span::hygiene::{ExpnKind, MacroKind}; use std::borrow::Cow; use std::cmp::{max, min, Reverse}; use std::io; @@ -346,15 +347,15 @@ pub trait Emitter { for (i, trace) in sp.macro_backtrace().iter().rev().enumerate() { // Only show macro locations that are local // and display them like a span_note - if trace.def_site_span.is_dummy() { + if trace.def_site.is_dummy() { continue; } if always_backtrace { new_labels.push(( - trace.def_site_span, + trace.def_site, format!( "in this expansion of `{}`{}", - trace.macro_decl_name, + trace.kind.descr(), if backtrace_len > 2 { // if backtrace_len == 1 it'll be pointed // at by "in this macro invocation" @@ -366,9 +367,8 @@ pub trait Emitter { )); } // Check to make sure we're not in any <*macros> - if !sm.span_to_filename(trace.def_site_span).is_macros() - && !trace.macro_decl_name.starts_with("desugaring of ") - && !trace.macro_decl_name.starts_with("#[") + if !sm.span_to_filename(trace.def_site).is_macros() + && matches!(trace.kind, ExpnKind::Macro(MacroKind::Bang, _)) || always_backtrace { new_labels.push(( diff --git a/src/librustc_errors/json.rs b/src/librustc_errors/json.rs index 29d3122636e..21be9527b6c 100644 --- a/src/librustc_errors/json.rs +++ b/src/librustc_errors/json.rs @@ -17,7 +17,8 @@ use crate::{Applicability, DiagnosticId}; use crate::{CodeSuggestion, SubDiagnostic}; use rustc_data_structures::sync::Lrc; -use rustc_span::{MacroBacktrace, MultiSpan, Span, SpanLabel}; +use rustc_span::hygiene::ExpnData; +use rustc_span::{MultiSpan, Span, SpanLabel}; use std::io::{self, Write}; use std::path::Path; use std::sync::{Arc, Mutex}; @@ -317,7 +318,7 @@ impl DiagnosticSpan { is_primary: bool, label: Option, suggestion: Option<(&String, Applicability)>, - mut backtrace: vec::IntoIter, + mut backtrace: vec::IntoIter, je: &JsonEmitter, ) -> DiagnosticSpan { let start = je.sm.lookup_char_pos(span.lo()); @@ -325,10 +326,10 @@ impl DiagnosticSpan { let backtrace_step = backtrace.next().map(|bt| { let call_site = Self::from_span_full(bt.call_site, false, None, None, backtrace, je); let def_site_span = - Self::from_span_full(bt.def_site_span, false, None, None, vec![].into_iter(), je); + Self::from_span_full(bt.def_site, false, None, None, vec![].into_iter(), je); Box::new(DiagnosticSpanMacroExpansion { span: call_site, - macro_decl_name: bt.macro_decl_name, + macro_decl_name: bt.kind.descr(), def_site_span, }) }); diff --git a/src/librustc_span/lib.rs b/src/librustc_span/lib.rs index 764312021ef..3f23eb15829 100644 --- a/src/librustc_span/lib.rs +++ b/src/librustc_span/lib.rs @@ -445,7 +445,7 @@ impl Span { self.ctxt().outer_expn_data().allow_internal_unsafe } - pub fn macro_backtrace(mut self) -> Vec { + pub fn macro_backtrace(mut self) -> Vec { let mut prev_span = DUMMY_SP; let mut result = vec![]; loop { @@ -455,11 +455,7 @@ impl Span { } // Don't print recursive invocations. if !expn_data.call_site.source_equal(&prev_span) { - result.push(MacroBacktrace { - call_site: expn_data.call_site, - macro_decl_name: expn_data.kind.descr(), - def_site_span: expn_data.def_site, - }); + result.push(expn_data.clone()); } prev_span = self; @@ -1501,18 +1497,6 @@ pub struct FileLines { pub static SPAN_DEBUG: AtomicRef) -> fmt::Result> = AtomicRef::new(&(default_span_debug as fn(_, &mut fmt::Formatter<'_>) -> _)); -#[derive(Debug)] -pub struct MacroBacktrace { - /// span where macro was applied to generate this code - pub call_site: Span, - - /// name of macro that was applied (e.g., "foo!" or "#[derive(Eq)]") - pub macro_decl_name: String, - - /// span where macro was defined (possibly dummy) - pub def_site_span: Span, -} - // _____________________________________________________________________________ // SpanLinesError, SpanSnippetError, DistinctSources, MalformedSourceMapPositions // From 6980f82c0d152446506fee4d4a45d8afdf4ad9a4 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 21 Jan 2020 01:46:53 +0200 Subject: [PATCH 4/4] rustc_span: return an impl Iterator instead of a Vec from macro_backtrace. --- src/librustc_errors/emitter.rs | 8 ++++---- src/librustc_errors/json.rs | 4 ++-- src/librustc_span/lib.rs | 33 ++++++++++++++++++--------------- src/librustc_span/source_map.rs | 3 +-- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 49c8be28292..f9e23e96fa8 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -343,8 +343,9 @@ pub trait Emitter { if call_sp != *sp && !always_backtrace { before_after.push((*sp, call_sp)); } - let backtrace_len = sp.macro_backtrace().len(); - for (i, trace) in sp.macro_backtrace().iter().rev().enumerate() { + let macro_backtrace: Vec<_> = sp.macro_backtrace().collect(); + let backtrace_len = macro_backtrace.len(); + for (i, trace) in macro_backtrace.iter().rev().enumerate() { // Only show macro locations that are local // and display them like a span_note if trace.def_site.is_dummy() { @@ -398,8 +399,7 @@ pub trait Emitter { continue; } if sm.span_to_filename(sp_label.span.clone()).is_macros() && !always_backtrace { - let v = sp_label.span.macro_backtrace(); - if let Some(use_site) = v.last() { + if let Some(use_site) = sp_label.span.macro_backtrace().last() { before_after.push((sp_label.span.clone(), use_site.call_site.clone())); } } diff --git a/src/librustc_errors/json.rs b/src/librustc_errors/json.rs index 21be9527b6c..3ddf9b09893 100644 --- a/src/librustc_errors/json.rs +++ b/src/librustc_errors/json.rs @@ -309,7 +309,7 @@ impl DiagnosticSpan { // 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 = span.macro_backtrace().into_iter(); + let backtrace = span.macro_backtrace(); DiagnosticSpan::from_span_full(span, is_primary, label, suggestion, backtrace, je) } @@ -318,7 +318,7 @@ impl DiagnosticSpan { is_primary: bool, label: Option, suggestion: Option<(&String, Applicability)>, - mut backtrace: vec::IntoIter, + mut backtrace: impl Iterator, je: &JsonEmitter, ) -> DiagnosticSpan { let start = je.sm.lookup_char_pos(span.lo()); diff --git a/src/librustc_span/lib.rs b/src/librustc_span/lib.rs index 3f23eb15829..413bd77daae 100644 --- a/src/librustc_span/lib.rs +++ b/src/librustc_span/lib.rs @@ -445,23 +445,26 @@ impl Span { self.ctxt().outer_expn_data().allow_internal_unsafe } - pub fn macro_backtrace(mut self) -> Vec { + pub fn macro_backtrace(mut self) -> impl Iterator { let mut prev_span = DUMMY_SP; - let mut result = vec![]; - loop { - let expn_data = self.ctxt().outer_expn_data(); - if expn_data.is_root() { - break; - } - // Don't print recursive invocations. - if !expn_data.call_site.source_equal(&prev_span) { - result.push(expn_data.clone()); - } + std::iter::from_fn(move || { + loop { + let expn_data = self.ctxt().outer_expn_data(); + if expn_data.is_root() { + return None; + } - prev_span = self; - self = expn_data.call_site; - } - result + let is_recursive = expn_data.call_site.source_equal(&prev_span); + + prev_span = self; + self = expn_data.call_site; + + // Don't print recursive invocations. + if !is_recursive { + return Some(expn_data); + } + } + }) } /// Returns a `Span` that would enclose both `self` and `end`. diff --git a/src/librustc_span/source_map.rs b/src/librustc_span/source_map.rs index e0b93b9ce25..c250df43a27 100644 --- a/src/librustc_span/source_map.rs +++ b/src/librustc_span/source_map.rs @@ -947,8 +947,7 @@ impl SourceMap { } pub fn call_span_if_macro(&self, sp: Span) -> Span { if self.span_to_filename(sp.clone()).is_macros() { - let v = sp.macro_backtrace(); - if let Some(use_site) = v.last() { + if let Some(use_site) = sp.macro_backtrace().last() { return use_site.call_site; } }