Skip to content
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

rustc_span: return an impl Iterator instead of a Vec from macro_backtrace. #68407

Merged
merged 4 commits into from
Jan 27, 2020
Merged
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
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