Auto merge of #68407 - eddyb:iter-macro-backtrace, r=petrochenkov

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

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;
@@ -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"
@@ -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((
@@ -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()));
                 }
             }

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};
@@ -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)
     }
 
@@ -317,7 +318,7 @@ 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());
@@ -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,
             })
         });

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",

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,

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,
         })
     }

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,
@@ -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()),
         }
     }
 }

src/librustc_span/lib.rs

@@ -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`.
@@ -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
 //

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;
             }
         }

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)*) };

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() {

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!())

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);
         }

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);

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!();

src/tools/cargo

@@ -1 +1 @@
-Subproject commit f6449ba236db31995255ac5e4cad4ab88296a7c6
+Subproject commit b68b0978ab8012f871c80736fb910d14b89c4498

src/tools/clippy

@@ -1 +1 @@
-Subproject commit 3e74853d1f9893cf2a47f28b658711d8f9f97b6b
+Subproject commit fa046d2e7f14cda09d14230cc8c772e1565e0757