Audit uses of apply_mark in built-in macros

petrochenkov · petrochenkov · commit 0fb01d219c7b · 2019-08-23T01:44:33.000+03:00
Replace them with equivalents of `Span::{def_site,call_site}` from proc macro API.
The new API is much less error prone and doesn't rely on macros having default transparency.
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
@@ -3,7 +3,7 @@ use crate::attr::{HasAttrs, Stability, Deprecation};
 use crate::source_map::SourceMap;
 use crate::edition::Edition;
 use crate::ext::expand::{self, AstFragment, Invocation};
-use crate::ext::hygiene::{ExpnId, SyntaxContext, Transparency};
+use crate::ext::hygiene::{ExpnId, Transparency};
 use crate::mut_visit::{self, MutVisitor};
 use crate::parse::{self, parser, DirectoryOwnership};
 use crate::parse::token;
@@ -760,23 +760,39 @@ impl<'a> ExtCtxt<'a> {
     pub fn call_site(&self) -> Span {
         self.current_expansion.id.expn_data().call_site
     }
-    pub fn backtrace(&self) -> SyntaxContext {
-        SyntaxContext::root().apply_mark(self.current_expansion.id)
+
+    /// Equivalent of `Span::def_site` from the proc macro API,
+    /// except that the location is taken from the span passed as an argument.
+    pub fn with_def_site_ctxt(&self, span: Span) -> Span {
+        span.with_ctxt_from_mark(self.current_expansion.id, Transparency::Opaque)
+    }
+
+    /// Equivalent of `Span::call_site` from the proc macro API,
+    /// except that the location is taken from the span passed as an argument.
+    pub fn with_call_site_ctxt(&self, span: Span) -> Span {
+        span.with_ctxt_from_mark(self.current_expansion.id, Transparency::Transparent)
+    }
+
+    /// Span with a context reproducing `macro_rules` hygiene (hygienic locals, unhygienic items).
+    /// FIXME: This should be eventually replaced either with `with_def_site_ctxt` (preferably),
+    /// or with `with_call_site_ctxt` (where necessary).
+    pub fn with_legacy_ctxt(&self, span: Span) -> Span {
+        span.with_ctxt_from_mark(self.current_expansion.id, Transparency::SemiTransparent)
     }
 
     /// Returns span for the macro which originally caused the current expansion to happen.
     ///
     /// Stops backtracing at include! boundary.
     pub fn expansion_cause(&self) -> Option<Span> {
-        let mut ctxt = self.backtrace();
+        let mut expn_id = self.current_expansion.id;
         let mut last_macro = None;
         loop {
-            let expn_data = ctxt.outer_expn_data();
+            let expn_data = expn_id.expn_data();
             // Stop going up the backtrace once include! is encountered
             if expn_data.is_root() || expn_data.kind.descr() == sym::include {
                 break;
             }
-            ctxt = expn_data.call_site.ctxt();
+            expn_id = expn_data.call_site.ctxt().outer_expn();
             last_macro = Some(expn_data.call_site);
         }
         last_macro
@@ -865,7 +881,7 @@ impl<'a> ExtCtxt<'a> {
         ast::Ident::from_str(st)
     }
     pub fn std_path(&self, components: &[Symbol]) -> Vec<ast::Ident> {
-        let def_site = DUMMY_SP.apply_mark(self.current_expansion.id);
+        let def_site = self.with_def_site_ctxt(DUMMY_SP);
         iter::once(Ident::new(kw::DollarCrate, def_site))
             .chain(components.iter().map(|&s| Ident::with_dummy_span(s)))
             .collect()
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
@@ -565,7 +565,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         return fragment_kind.dummy(span);
                     }
                     let meta = ast::MetaItem { node: ast::MetaItemKind::Word, span, path };
-                    let span = span.with_ctxt(self.cx.backtrace());
                     let items = expander.expand(self.cx, span, &meta, item);
                     fragment_kind.expect_from_annotatables(items)
                 }
diff --git a/src/libsyntax/ext/proc_macro_server.rs b/src/libsyntax/ext/proc_macro_server.rs
@@ -7,7 +7,6 @@ use crate::tokenstream::{self, DelimSpan, IsJoint::*, TokenStream, TreeAndJoint}
 use errors::{Diagnostic, DiagnosticBuilder};
 use rustc_data_structures::sync::Lrc;
 use syntax_pos::{BytePos, FileName, MultiSpan, Pos, SourceFile, Span};
-use syntax_pos::hygiene::{SyntaxContext, Transparency};
 use syntax_pos::symbol::{kw, sym, Symbol};
 
 use proc_macro::{Delimiter, Level, LineColumn, Spacing};
@@ -363,16 +362,10 @@ impl<'a> Rustc<'a> {
     pub fn new(cx: &'a ExtCtxt<'_>) -> Self {
         // No way to determine def location for a proc macro right now, so use call location.
         let location = cx.current_expansion.id.expn_data().call_site;
-        let to_span = |transparency| {
-            location.with_ctxt(
-                SyntaxContext::root()
-                    .apply_mark_with_transparency(cx.current_expansion.id, transparency),
-            )
-        };
         Rustc {
             sess: cx.parse_sess,
-            def_site: to_span(Transparency::Opaque),
-            call_site: to_span(Transparency::Transparent),
+            def_site: cx.with_def_site_ctxt(location),
+            call_site: cx.with_call_site_ctxt(location),
         }
     }
 
diff --git a/src/libsyntax_ext/asm.rs b/src/libsyntax_ext/asm.rs
@@ -63,7 +63,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt<'_>,
     MacEager::expr(P(ast::Expr {
         id: ast::DUMMY_NODE_ID,
         node: ast::ExprKind::InlineAsm(P(inline_asm)),
-        span: sp.with_ctxt(cx.backtrace()),
+        span: cx.with_legacy_ctxt(sp),
         attrs: ThinVec::new(),
     }))
 }
diff --git a/src/libsyntax_ext/assert.rs b/src/libsyntax_ext/assert.rs
@@ -23,7 +23,7 @@ pub fn expand_assert<'cx>(
         }
     };
 
-    let sp = sp.apply_mark(cx.current_expansion.id);
+    let sp = cx.with_legacy_ctxt(sp);
     let panic_call = Mac {
         path: Path::from_ident(Ident::new(sym::panic, sp)),
         tts: custom_message.unwrap_or_else(|| {
diff --git a/src/libsyntax_ext/cfg.rs b/src/libsyntax_ext/cfg.rs
@@ -16,7 +16,7 @@ pub fn expand_cfg(
     sp: Span,
     tts: &[tokenstream::TokenTree],
 ) -> Box<dyn base::MacResult + 'static> {
-    let sp = sp.apply_mark(cx.current_expansion.id);
+    let sp = cx.with_legacy_ctxt(sp);
 
     match parse_cfg(cx, sp, tts) {
         Ok(cfg) => {
diff --git a/src/libsyntax_ext/concat.rs b/src/libsyntax_ext/concat.rs
@@ -59,6 +59,6 @@ pub fn expand_syntax_ext(
     } else if has_errors {
         return DummyResult::any(sp);
     }
-    let sp = sp.apply_mark(cx.current_expansion.id);
+    let sp = cx.with_legacy_ctxt(sp);
     base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))
 }
diff --git a/src/libsyntax_ext/concat_idents.rs b/src/libsyntax_ext/concat_idents.rs
@@ -39,7 +39,7 @@ pub fn expand_syntax_ext<'cx>(cx: &'cx mut ExtCtxt<'_>,
         }
     }
 
-    let ident = ast::Ident::new(Symbol::intern(&res_str), sp.apply_mark(cx.current_expansion.id));
+    let ident = ast::Ident::new(Symbol::intern(&res_str), cx.with_legacy_ctxt(sp));
 
     struct ConcatIdentsResult { ident: ast::Ident }
 
diff --git a/src/libsyntax_ext/deriving/clone.rs b/src/libsyntax_ext/deriving/clone.rs
@@ -112,7 +112,7 @@ fn cs_clone_shallow(name: &str,
                         ty: P<ast::Ty>, span: Span, helper_name: &str) {
         // Generate statement `let _: helper_name<ty>;`,
         // set the expn ID so we can use the unstable struct.
-        let span = span.with_ctxt(cx.backtrace());
+        let span = cx.with_def_site_ctxt(span);
         let assert_path = cx.path_all(span, true,
                                         cx.std_path(&[sym::clone, Symbol::intern(helper_name)]),
                                         vec![GenericArg::Type(ty)], vec![]);
diff --git a/src/libsyntax_ext/deriving/cmp/eq.rs b/src/libsyntax_ext/deriving/cmp/eq.rs
@@ -53,7 +53,7 @@ fn cs_total_eq_assert(cx: &mut ExtCtxt<'_>,
                         ty: P<ast::Ty>, span: Span, helper_name: &str) {
         // Generate statement `let _: helper_name<ty>;`,
         // set the expn ID so we can use the unstable struct.
-        let span = span.with_ctxt(cx.backtrace());
+        let span = cx.with_def_site_ctxt(span);
         let assert_path = cx.path_all(span, true,
                                         cx.std_path(&[sym::cmp, Symbol::intern(helper_name)]),
                                         vec![GenericArg::Type(ty)], vec![]);
diff --git a/src/libsyntax_ext/deriving/debug.rs b/src/libsyntax_ext/deriving/debug.rs
@@ -60,7 +60,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
     };
 
     // We want to make sure we have the ctxt set so that we can use unstable methods
-    let span = span.with_ctxt(cx.backtrace());
+    let span = cx.with_def_site_ctxt(span);
     let name = cx.expr_lit(span, ast::LitKind::Str(ident.name, ast::StrStyle::Cooked));
     let builder = Ident::from_str_and_span("debug_trait_builder", span);
     let builder_expr = cx.expr_ident(span, builder.clone());
diff --git a/src/libsyntax_ext/deriving/generic/ty.rs b/src/libsyntax_ext/deriving/generic/ty.rs
@@ -85,7 +85,7 @@ impl<'a> Path<'a> {
             PathKind::Global => cx.path_all(span, true, idents, params, Vec::new()),
             PathKind::Local => cx.path_all(span, false, idents, params, Vec::new()),
             PathKind::Std => {
-                let def_site = DUMMY_SP.apply_mark(cx.current_expansion.id);
+                let def_site = cx.with_def_site_ctxt(DUMMY_SP);
                 idents.insert(0, Ident::new(kw::DollarCrate, def_site));
                 cx.path_all(span, false, idents, params, Vec::new())
             }
diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs
@@ -48,6 +48,9 @@ impl MultiItemModifier for BuiltinDerive {
               meta_item: &MetaItem,
               item: Annotatable)
               -> Vec<Annotatable> {
+        // FIXME: Built-in derives often forget to give spans contexts,
+        // so we are doing it here in a centralized way.
+        let span = ecx.with_def_site_ctxt(span);
         let mut items = Vec::new();
         (self.0)(ecx, span, meta_item, &item, &mut |a| items.push(a));
         items
@@ -60,7 +63,7 @@ fn call_intrinsic(cx: &ExtCtxt<'_>,
                   intrinsic: &str,
                   args: Vec<P<ast::Expr>>)
                   -> P<ast::Expr> {
-    let span = span.with_ctxt(cx.backtrace());
+    let span = cx.with_def_site_ctxt(span);
     let path = cx.std_path(&[sym::intrinsics, Symbol::intern(intrinsic)]);
     let call = cx.expr_call_global(span, path, args);
 
diff --git a/src/libsyntax_ext/env.rs b/src/libsyntax_ext/env.rs
@@ -20,7 +20,7 @@ pub fn expand_option_env<'cx>(cx: &'cx mut ExtCtxt<'_>,
         Some(v) => v,
     };
 
-    let sp = sp.apply_mark(cx.current_expansion.id);
+    let sp = cx.with_legacy_ctxt(sp);
     let e = match env::var(&*var.as_str()) {
         Err(..) => {
             let lt = cx.lifetime(sp, Ident::with_dummy_span(kw::StaticLifetime));
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
@@ -12,7 +12,7 @@ use syntax::parse::token;
 use syntax::ptr::P;
 use syntax::symbol::{Symbol, sym};
 use syntax::tokenstream;
-use syntax_pos::{MultiSpan, Span, DUMMY_SP};
+use syntax_pos::{MultiSpan, Span};
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use std::borrow::Cow;
@@ -666,8 +666,7 @@ impl<'a, 'b> Context<'a, 'b> {
         // passed to this function.
         for (i, e) in self.args.into_iter().enumerate() {
             let name = names_pos[i];
-            let span =
-                DUMMY_SP.with_ctxt(e.span.ctxt().apply_mark(self.ecx.current_expansion.id));
+            let span = self.ecx.with_def_site_ctxt(e.span);
             pats.push(self.ecx.pat_ident(span, name));
             for ref arg_ty in self.arg_unique_types[i].iter() {
                 locals.push(Context::format_arg(self.ecx, self.macsp, e.span, arg_ty, name));
@@ -745,7 +744,7 @@ impl<'a, 'b> Context<'a, 'b> {
         ty: &ArgumentType,
         arg: ast::Ident,
     ) -> P<ast::Expr> {
-        sp = sp.apply_mark(ecx.current_expansion.id);
+        sp = ecx.with_def_site_ctxt(sp);
         let arg = ecx.expr_ident(sp, arg);
         let trait_ = match *ty {
             Placeholder(ref tyname) => {
@@ -798,7 +797,7 @@ fn expand_format_args_impl<'cx>(
     tts: &[tokenstream::TokenTree],
     nl: bool,
 ) -> Box<dyn base::MacResult + 'cx> {
-    sp = sp.apply_mark(ecx.current_expansion.id);
+    sp = ecx.with_def_site_ctxt(sp);
     match parse_args(ecx, sp, tts) {
         Ok((efmt, args, names)) => {
             MacEager::expr(expand_preparsed_format_args(ecx, sp, efmt, args, names, nl))
@@ -842,7 +841,7 @@ pub fn expand_preparsed_format_args(
     let arg_unique_types: Vec<_> = (0..args.len()).map(|_| Vec::new()).collect();
 
     let mut macsp = ecx.call_site();
-    macsp = macsp.with_ctxt(ecx.backtrace());
+    macsp = ecx.with_def_site_ctxt(macsp);
 
     let msg = "format argument must be a string literal";
     let fmt_sp = efmt.span;
diff --git a/src/libsyntax_ext/global_allocator.rs b/src/libsyntax_ext/global_allocator.rs
@@ -3,7 +3,6 @@ use syntax::ast::{self, Arg, Attribute, Expr, FnHeader, Generics, Ident};
 use syntax::attr::check_builtin_macro_attribute;
 use syntax::ext::allocator::{AllocatorKind, AllocatorMethod, AllocatorTy, ALLOCATOR_METHODS};
 use syntax::ext::base::{Annotatable, ExtCtxt};
-use syntax::ext::hygiene::SyntaxContext;
 use syntax::ptr::P;
 use syntax::symbol::{kw, sym, Symbol};
 use syntax_pos::Span;
@@ -29,7 +28,7 @@ pub fn expand(
     };
 
     // Generate a bunch of new items using the AllocFnFactory
-    let span = item.span.with_ctxt(SyntaxContext::root().apply_mark(ecx.current_expansion.id));
+    let span = ecx.with_legacy_ctxt(item.span);
     let f = AllocFnFactory {
         span,
         kind: AllocatorKind::Global,
diff --git a/src/libsyntax_ext/global_asm.rs b/src/libsyntax_ext/global_asm.rs
@@ -30,7 +30,7 @@ pub fn expand_global_asm<'cx>(cx: &'cx mut ExtCtxt<'_>,
                 id: ast::DUMMY_NODE_ID,
                 node: ast::ItemKind::GlobalAsm(P(global_asm)),
                 vis: respan(sp.shrink_to_lo(), ast::VisibilityKind::Inherited),
-                span: sp.with_ctxt(cx.backtrace()),
+                span: cx.with_legacy_ctxt(sp),
                 tokens: None,
             })])
         }
diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs
@@ -4,7 +4,6 @@
 use syntax::ast;
 use syntax::attr::{self, check_builtin_macro_attribute};
 use syntax::ext::base::*;
-use syntax::ext::hygiene::SyntaxContext;
 use syntax::print::pprust;
 use syntax::source_map::respan;
 use syntax::symbol::{Symbol, sym};
@@ -29,7 +28,7 @@ pub fn expand_test_case(
 
     if !ecx.ecfg.should_test { return vec![]; }
 
-    let sp = attr_sp.with_ctxt(SyntaxContext::root().apply_mark(ecx.current_expansion.id));
+    let sp = ecx.with_legacy_ctxt(attr_sp);
     let mut item = anno_item.expect_item();
     item = item.map(|mut item| {
         item.vis = respan(item.vis.span, ast::VisibilityKind::Public);
@@ -93,8 +92,7 @@ pub fn expand_test_or_bench(
         return vec![Annotatable::Item(item)];
     }
 
-    let ctxt = SyntaxContext::root().apply_mark(cx.current_expansion.id);
-    let (sp, attr_sp) = (item.span.with_ctxt(ctxt), attr_sp.with_ctxt(ctxt));
+    let (sp, attr_sp) = (cx.with_legacy_ctxt(item.span), cx.with_legacy_ctxt(attr_sp));
 
     // Gensym "test" so we can extern crate without conflicting with any local names
     let test_id = cx.ident_of("test").gensym();
diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
@@ -22,6 +22,7 @@ pub mod edition;
 use edition::Edition;
 pub mod hygiene;
 pub use hygiene::{ExpnId, SyntaxContext, ExpnData, ExpnKind, MacroKind, DesugaringKind};
+use hygiene::Transparency;
 
 mod span_encoding;
 pub use span_encoding::{Span, DUMMY_SP};
@@ -512,6 +513,13 @@ impl Span {
                   span.ctxt)
     }
 
+    /// Produces a span with the same location as `self` and context produced by a macro with the
+    /// given ID and transparency, assuming that macro was defined directly and not produced by
+    /// some other macro (which is the case for built-in and procedural macros).
+    pub fn with_ctxt_from_mark(self, expn_id: ExpnId, transparency: Transparency) -> Span {
+        self.with_ctxt(SyntaxContext::root().apply_mark_with_transparency(expn_id, transparency))
+    }
+
     #[inline]
     pub fn apply_mark(self, mark: ExpnId) -> Span {
         let span = self.data();

Original file line number	Diff line number	Diff line change
`@@ -565,7 +565,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {`
`565`	`565`	`return fragment_kind.dummy(span);`
`566`	`566`	`}`
`567`	`567`	`let meta = ast::MetaItem { node: ast::MetaItemKind::Word, span, path };`
`568`		`- let span = span.with_ctxt(self.cx.backtrace());`
`569`	`568`	`let items = expander.expand(self.cx, span, &meta, item);`
`570`	`569`	`fragment_kind.expect_from_annotatables(items)`
`571`	`570`	`}`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt<'_>,`
`63`	`63`	`MacEager::expr(P(ast::Expr {`
`64`	`64`	`id: ast::DUMMY_NODE_ID,`
`65`	`65`	`node: ast::ExprKind::InlineAsm(P(inline_asm)),`
`66`		`- span: sp.with_ctxt(cx.backtrace()),`
	`66`	`+ span: cx.with_legacy_ctxt(sp),`
`67`	`67`	`attrs: ThinVec::new(),`
`68`	`68`	`}))`
`69`	`69`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ pub fn expand_assert<'cx>(`
`23`	`23`	`}`
`24`	`24`	`};`
`25`	`25`
`26`		`- let sp = sp.apply_mark(cx.current_expansion.id);`
	`26`	`+ let sp = cx.with_legacy_ctxt(sp);`
`27`	`27`	`let panic_call = Mac {`
`28`	`28`	`path: Path::from_ident(Ident::new(sym::panic, sp)),`
`29`	`29`	`tts: custom_message.unwrap_or_else(\|\| {`
Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,6 @@ pub fn expand_syntax_ext(`
`59`	`59`	`} else if has_errors {`
`60`	`60`	`return DummyResult::any(sp);`
`61`	`61`	`}`
`62`		`- let sp = sp.apply_mark(cx.current_expansion.id);`
	`62`	`+ let sp = cx.with_legacy_ctxt(sp);`
`63`	`63`	`base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))`
`64`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ pub fn expand_syntax_ext<'cx>(cx: &'cx mut ExtCtxt<'_>,`
`39`	`39`	`}`
`40`	`40`	`}`
`41`	`41`
`42`		`- let ident = ast::Ident::new(Symbol::intern(&res_str), sp.apply_mark(cx.current_expansion.id));`
	`42`	`+ let ident = ast::Ident::new(Symbol::intern(&res_str), cx.with_legacy_ctxt(sp));`
`43`	`43`
`44`	`44`	`struct ConcatIdentsResult { ident: ast::Ident }`
`45`	`45`