syntax: Make def-site span mandatory in ExpnInfo/MacroBacktrace/DiagnosticSpanMacroExpansion

We have to deal with dummy spans anyway

Remove def-site span from expander interfaces.
It's not used by the expansion infra, only by specific expanders, which can keep it themselves if they want it.

Author: petrochenkov

src/librustc/hir/lowering.rs

@@ -877,7 +877,7 @@ impl<'a> LoweringContext<'a> {
     ) -> Span {
         let mark = Mark::fresh(Mark::root());
         mark.set_expn_info(ExpnInfo {
-            def_site: Some(span),
+            def_site: span,
             allow_internal_unstable,
             ..ExpnInfo::default(ExpnKind::Desugaring(reason), span, self.sess.edition())
         });

src/librustc/lint/mod.rs

@@ -888,13 +888,11 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
         ExpnKind::Desugaring(DesugaringKind::ForLoop) => false,
         ExpnKind::Desugaring(_) => true, // well, it's "external"
         ExpnKind::MacroBang(..) => {
-            let def_site = match info.def_site {
-                Some(span) => span,
-                // no span for the def_site means it's an external macro
-                None => return true,
-            };
-
-            match sess.source_map().span_to_snippet(def_site) {
+            if info.def_site.is_dummy() {
+                // dummy span for the def_site means it's an external macro
+                return true;
+            }
+            match sess.source_map().span_to_snippet(info.def_site) {
                 Ok(code) => !code.starts_with("macro_rules"),
                 // no snippet = external macro or compiler-builtin expansion
                 Err(_) => true,

src/librustc/traits/error_reporting.rs

@@ -61,12 +61,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             // We want to ignore desugarings here: spans are equivalent even
             // if one is the result of a desugaring and the other is not.
             let mut span = error.obligation.cause.span;
-            if let Some(ExpnInfo {
-                kind: ExpnKind::Desugaring(_),
-                def_site: Some(def_span),
-                ..
-            }) = span.ctxt().outer_expn_info() {
-                span = def_span;
+            if let Some(ExpnInfo { kind: ExpnKind::Desugaring(_), def_site, .. })
+                    = span.ctxt().outer_expn_info() {
+                span = def_site;
             }
 
             error_map.entry(span).or_default().push(

src/librustc_errors/emitter.rs

@@ -723,39 +723,37 @@ impl EmitterWriter {
                 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 let Some(def_site) = trace.def_site_span {
-                        if def_site.is_dummy() {
-                            continue;
-                        }
-                        if always_backtrace {
-                            new_labels.push((def_site,
-                                             format!("in this expansion of `{}`{}",
-                                                     trace.macro_decl_name,
-                                                     if backtrace_len > 2 {
-                                                         // if backtrace_len == 1 it'll be pointed
-                                                         // at by "in this macro invocation"
-                                                         format!(" (#{})", i + 1)
-                                                     } else {
-                                                         String::new()
-                                                     })));
-                        }
-                        // Check to make sure we're not in any <*macros>
-                        if !sm.span_to_filename(def_site).is_macros() &&
-                           !trace.macro_decl_name.starts_with("desugaring of ") &&
-                           !trace.macro_decl_name.starts_with("#[") ||
-                           always_backtrace {
-                            new_labels.push((trace.call_site,
-                                             format!("in this macro invocation{}",
-                                                     if backtrace_len > 2 && always_backtrace {
-                                                         // only specify order when the macro
-                                                         // backtrace is multiple levels deep
-                                                         format!(" (#{})", i + 1)
-                                                     } else {
-                                                         String::new()
-                                                     })));
-                            if !always_backtrace {
-                                break;
-                            }
+                    if trace.def_site_span.is_dummy() {
+                        continue;
+                    }
+                    if always_backtrace {
+                        new_labels.push((trace.def_site_span,
+                                            format!("in this expansion of `{}`{}",
+                                                    trace.macro_decl_name,
+                                                    if backtrace_len > 2 {
+                                                        // if backtrace_len == 1 it'll be pointed
+                                                        // at by "in this macro invocation"
+                                                        format!(" (#{})", i + 1)
+                                                    } else {
+                                                        String::new()
+                                                    })));
+                    }
+                    // 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("#[") ||
+                        always_backtrace {
+                        new_labels.push((trace.call_site,
+                                            format!("in this macro invocation{}",
+                                                    if backtrace_len > 2 && always_backtrace {
+                                                        // only specify order when the macro
+                                                        // backtrace is multiple levels deep
+                                                        format!(" (#{})", i + 1)
+                                                    } else {
+                                                        String::new()
+                                                    })));
+                        if !always_backtrace {
+                            break;
                         }
                     }
                 }

src/librustc_save_analysis/lib.rs

@@ -841,7 +841,6 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
         let callsite = span.source_callsite();
         let callsite_span = self.span_from_span(callsite);
         let callee = span.source_callee()?;
-        let callee_span = callee.def_site?;
 
         // Ignore attribute macros, their spans are usually mangled
         if let ExpnKind::MacroAttribute(_) = callee.kind {
@@ -855,7 +854,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
             .sess
             .imported_macro_spans
             .borrow()
-            .get(&callee_span)
+            .get(&callee.def_site)
         {
             let &(ref mac_name, mac_span) = mac;
             let mac_span = self.span_from_span(mac_span);
@@ -866,7 +865,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
             });
         }
 
-        let callee_span = self.span_from_span(callee_span);
+        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

src/libsyntax/ext/base.rs

@@ -219,7 +219,6 @@ pub trait TTMacroExpander {
         ecx: &'cx mut ExtCtxt<'_>,
         span: Span,
         input: TokenStream,
-        def_span: Option<Span>,
     ) -> Box<dyn MacResult+'cx>;
 }
 
@@ -236,7 +235,6 @@ impl<F> TTMacroExpander for F
         ecx: &'cx mut ExtCtxt<'_>,
         span: Span,
         input: TokenStream,
-        _def_span: Option<Span>,
     ) -> Box<dyn MacResult+'cx> {
         struct AvoidInterpolatedIdents;
 
@@ -654,7 +652,7 @@ impl SyntaxExtension {
         ExpnInfo {
             call_site,
             kind: self.expn_kind(Symbol::intern(descr)),
-            def_site: Some(self.span),
+            def_site: self.span,
             default_transparency: self.default_transparency,
             allow_internal_unstable: self.allow_internal_unstable.clone(),
             allow_internal_unsafe: self.allow_internal_unsafe,

src/libsyntax/ext/expand.rs

@@ -673,7 +673,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 result
             }
             SyntaxExtensionKind::LegacyBang(expander) => {
-                let tok_result = expander.expand(self.cx, span, mac.node.stream(), Some(ext.span));
+                let tok_result = expander.expand(self.cx, span, mac.node.stream());
                 kind.make_from(tok_result)
             }
 

src/libsyntax/ext/tt/macro_rules.rs

@@ -88,6 +88,7 @@ impl<'a> ParserAnyMacro<'a> {
 
 struct MacroRulesMacroExpander {
     name: ast::Ident,
+    span: Span,
     lhses: Vec<quoted::TokenTree>,
     rhses: Vec<quoted::TokenTree>,
     valid: bool,
@@ -99,12 +100,11 @@ impl TTMacroExpander for MacroRulesMacroExpander {
         cx: &'cx mut ExtCtxt<'_>,
         sp: Span,
         input: TokenStream,
-        def_span: Option<Span>,
     ) -> Box<dyn MacResult + 'cx> {
         if !self.valid {
             return DummyResult::any(sp);
         }
-        generic_extension(cx, sp, def_span, self.name, input, &self.lhses, &self.rhses)
+        generic_extension(cx, sp, self.span, self.name, input, &self.lhses, &self.rhses)
     }
 }
 
@@ -117,7 +117,7 @@ fn trace_macros_note(cx: &mut ExtCtxt<'_>, sp: Span, message: String) {
 fn generic_extension<'cx>(
     cx: &'cx mut ExtCtxt<'_>,
     sp: Span,
-    def_span: Option<Span>,
+    def_span: Span,
     name: ast::Ident,
     arg: TokenStream,
     lhses: &[quoted::TokenTree],
@@ -199,10 +199,8 @@ fn generic_extension<'cx>(
     let span = token.span.substitute_dummy(sp);
     let mut err = cx.struct_span_err(span, &parse_failure_msg(&token));
     err.span_label(span, label);
-    if let Some(sp) = def_span {
-        if cx.source_map().span_to_filename(sp).is_real() && !sp.is_dummy() {
-            err.span_label(cx.source_map().def_span(sp), "when calling this macro");
-        }
+    if !def_span.is_dummy() && cx.source_map().span_to_filename(def_span).is_real() {
+        err.span_label(cx.source_map().def_span(def_span), "when calling this macro");
     }
 
     // Check whether there's a missing comma in this macro call, like `println!("{}" a);`
@@ -377,7 +375,7 @@ pub fn compile(
     }
 
     let expander: Box<_> =
-        Box::new(MacroRulesMacroExpander { name: def.ident, lhses, rhses, valid });
+        Box::new(MacroRulesMacroExpander { name: def.ident, span: def.span, lhses, rhses, valid });
 
     let (default_transparency, transparency_error) =
         attr::find_transparency(&def.attrs, body.legacy);

src/libsyntax/json.rs

@@ -170,7 +170,7 @@ struct DiagnosticSpanMacroExpansion {
     macro_decl_name: String,
 
     /// span where macro was defined (if known)
-    def_site_span: Option<DiagnosticSpan>,
+    def_site_span: DiagnosticSpan,
 }
 
 #[derive(RustcEncodable)]
@@ -300,14 +300,13 @@ impl DiagnosticSpan {
                                      None,
                                      backtrace,
                                      je);
-            let def_site_span = bt.def_site_span.map(|sp| {
-                Self::from_span_full(sp,
+            let def_site_span =
+                Self::from_span_full(bt.def_site_span,
                                      false,
                                      None,
                                      None,
                                      vec![].into_iter(),
-                                     je)
-            });
+                                     je);
             Box::new(DiagnosticSpanMacroExpansion {
                 span: call_site,
                 macro_decl_name: bt.macro_decl_name,

src/libsyntax_pos/hygiene.rs

@@ -26,7 +26,7 @@
 // trigger runtime aborts. (Fortunately these are obvious and easy to fix.)
 
 use crate::GLOBALS;
-use crate::Span;
+use crate::{Span, DUMMY_SP};
 use crate::edition::Edition;
 use crate::symbol::{kw, Symbol};
 
@@ -632,11 +632,9 @@ pub struct ExpnInfo {
 
     // --- The part specific to the macro/desugaring definition.
     // --- FIXME: Share it between expansions with the same definition.
-    /// The span of the macro definition itself. The macro may not
-    /// have a sensible definition span (e.g., something defined
-    /// completely inside libsyntax) in which case this is None.
+    /// The span of the macro definition (possibly dummy).
     /// This span serves only informational purpose and is not used for resolution.
-    pub def_site: Option<Span>,
+    pub def_site: Span,
     /// Transparency used by `apply_mark` for mark with this expansion info by default.
     pub default_transparency: Transparency,
     /// List of #[unstable]/feature-gated features that the macro is allowed to use
@@ -659,7 +657,7 @@ impl ExpnInfo {
         ExpnInfo {
             call_site,
             kind,
-            def_site: None,
+            def_site: DUMMY_SP,
             default_transparency: Transparency::SemiTransparent,
             allow_internal_unstable: None,
             allow_internal_unsafe: false,