Skip to content

Commit

Permalink
Auto merge of #68407 - eddyb:iter-macro-backtrace, r=petrochenkov
Browse files Browse the repository at this point in the history
rustc_span: return an impl Iterator instead of a Vec from macro_backtrace.

Having `Span::macro_backtrace` produce an `impl Iterator<Item = ExpnData>` allows #67359 to use it instead of rolling its own similar functionality.

The move from `MacroBacktrace` to `ExpnData` (which the first two commits are prerequisites for) both eliminates unnecessary allocations, and is strictly more flexible (exposes more information).

r? @petrochenkov
  • Loading branch information
bors committed Jan 26, 2020
2 parents 698fcd3 + 6980f82 commit a237641
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 86 deletions.
20 changes: 10 additions & 10 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -342,19 +343,20 @@ 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_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"
Expand All @@ -366,9 +368,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((
Expand Down Expand Up @@ -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()));
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/librustc_errors/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -308,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)
}

Expand All @@ -317,18 +318,18 @@ impl DiagnosticSpan {
is_primary: bool,
label: Option<String>,
suggestion: Option<(&String, Applicability)>,
mut backtrace: vec::IntoIter<MacroBacktrace>,
mut backtrace: impl Iterator<Item = ExpnData>,
je: &JsonEmitter,
) -> DiagnosticSpan {
let start = je.sm.lookup_char_pos(span.lo());
let end = je.sm.lookup_char_pos(span.hi());
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,
})
});
Expand Down
5 changes: 1 addition & 4 deletions src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_lint/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand Down
21 changes: 14 additions & 7 deletions src/librustc_save_analysis/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
})
}
Expand Down
20 changes: 13 additions & 7 deletions src/librustc_span/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -730,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()),
}
}
}
Expand Down
59 changes: 18 additions & 41 deletions src/librustc_span/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,37 +445,26 @@ impl Span {
self.ctxt().outer_expn_data().allow_internal_unsafe
}

pub fn macro_backtrace(mut self) -> Vec<MacroBacktrace> {
pub fn macro_backtrace(mut self) -> impl Iterator<Item = ExpnData> {
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) {
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),
def_site_span: expn_data.def_site,
});
}
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`.
Expand Down Expand Up @@ -1511,18 +1500,6 @@ pub struct FileLines {
pub static SPAN_DEBUG: AtomicRef<fn(Span, &mut fmt::Formatter<'_>) -> 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
//
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_span/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/did_you_mean/recursion_limit_macro.stderr
Original file line number Diff line number Diff line change
@@ -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)*) };
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/infinite/infinite-macro-expansion.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/infinite/infinite-macro-expansion.stderr
Original file line number Diff line number Diff line change
@@ -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!())
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-16098.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-16098.stderr
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/macros/trace_faulty_macros.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy
Submodule clippy updated 45 files
+1 −0 CHANGELOG.md
+1 −1 README.md
+18 −4 clippy_lints/src/empty_enum.rs
+13 −1 clippy_lints/src/eq_op.rs
+22 −16 clippy_lints/src/formatting.rs
+5 −0 clippy_lints/src/let_underscore.rs
+5 −1 clippy_lints/src/lib.rs
+4 −4 clippy_lints/src/loops.rs
+2 −10 clippy_lints/src/main_recursion.rs
+102 −1 clippy_lints/src/methods/mod.rs
+199 −181 clippy_lints/src/types.rs
+15 −10 clippy_lints/src/unused_io_amount.rs
+2 −0 clippy_lints/src/utils/conf.rs
+24 −14 clippy_lints/src/utils/mod.rs
+8 −0 clippy_lints/src/utils/paths.rs
+8 −1 src/lintlist/mod.rs
+1 −1 tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
+1 −0 tests/ui-toml/vec_box_sized/clippy.toml
+15 −0 tests/ui-toml/vec_box_sized/test.rs
+22 −0 tests/ui-toml/vec_box_sized/test.stderr
+1 −1 tests/ui/empty_enum.stderr
+9 −0 tests/ui/eq_op.rs
+11 −0 tests/ui/formatting.rs
+9 −1 tests/ui/formatting.stderr
+22 −0 tests/ui/issue-3746.rs
+7 −0 tests/ui/let_underscore.rs
+12 −12 tests/ui/let_underscore.stderr
+0 −90 tests/ui/match_same_arms.rs
+26 −132 tests/ui/match_same_arms.stderr
+84 −0 tests/ui/match_same_arms2.rs
+109 −0 tests/ui/match_same_arms2.stderr
+1 −2 tests/ui/missing_const_for_fn/could_be_const.rs
+5 −5 tests/ui/missing_const_for_fn/could_be_const.stderr
+2 −85 tests/ui/needless_range_loop.rs
+16 −104 tests/ui/needless_range_loop.stderr
+85 −0 tests/ui/needless_range_loop2.rs
+91 −0 tests/ui/needless_range_loop2.stderr
+38 −0 tests/ui/option_as_ref_deref.fixed
+41 −0 tests/ui/option_as_ref_deref.rs
+92 −0 tests/ui/option_as_ref_deref.stderr
+6 −0 tests/ui/unused_io_amount.rs
+17 −5 tests/ui/unused_io_amount.stderr
+3 −1 tests/ui/vec_box_sized.fixed
+3 −1 tests/ui/vec_box_sized.rs
+3 −3 tests/ui/vec_box_sized.stderr

0 comments on commit a237641

Please sign in to comment.