Auto merge of rust-lang#63823 - petrochenkov:noapply2, r=matthewjasper

Audit uses of `apply_mark` in built-in macros + Remove default macro transparencies

Every use of `apply_mark` in a built-in or procedural macro is supposed to look like this
```rust
location.with_ctxt(SyntaxContext::root().apply_mark(ecx.current_expansion.id))
```
where `SyntaxContext::root()` means that the built-in/procedural macro is defined directly, rather than expanded from some other macro.

However, few people understood what `apply_mark` does, so we had a lot of copy-pasted uses of it looking e.g. like
```rust
span = span.apply_mark(ecx.current_expansion.id);
```
, which doesn't really make sense for procedural macros, but at the same time is not too harmful, if the macros use the traditional `macro_rules` hygiene.

So, to fight this, we stop using `apply_mark` directly in built-in macro implementations, and follow the example of regular proc macros instead and use analogues of `Span::def_site()` and `Span::call_site()`, which are much more intuitive and less error-prone.
- `ecx.with_def_site_ctxt(span)` takes the `span`'s location and combines it with a def-site context.
- `ecx.with_call_site_ctxt(span)` takes the `span`'s location and combines it with a call-site context.

Even if called multiple times (which sometimes happens due to some historical messiness of the built-in macro code) these functions will produce the same result, unlike `apply_mark` which will grow  the mark chain further in this case.

---

After `apply_mark`s in built-in macros are eliminated, the remaining `apply_mark`s are very few in number, so we can start passing the previously implicit `Transparency` argument to them explicitly, thus eliminating the need in `default_transparency` fields in hygiene structures and `#[rustc_macro_transparency]` annotations on built-in macros.

So, the task of making built-in macros opaque can now be formulated as "eliminate `with_legacy_ctxt` in favor of `with_def_site_ctxt`" rather than "replace `#[rustc_macro_transparency = "semitransparent"]` with `#[rustc_macro_transparency = "opaque"]`".

r? @matthewjasper

Author: bors

src/libcore/macros.rs

@@ -734,7 +734,6 @@ pub(crate) mod builtin {
     #[allow_internal_unstable(fmt_internals)]
     #[rustc_builtin_macro]
     #[macro_export]
-    #[rustc_macro_transparency = "opaque"]
     macro_rules! format_args {
         ($fmt:expr) => ({ /* compiler built-in */ });
         ($fmt:expr, $($args:tt)*) => ({ /* compiler built-in */ })
@@ -747,7 +746,6 @@ pub(crate) mod builtin {
     #[allow_internal_unstable(fmt_internals)]
     #[rustc_builtin_macro]
     #[macro_export]
-    #[rustc_macro_transparency = "opaque"]
     macro_rules! format_args_nl {
         ($fmt:expr) => ({ /* compiler built-in */ });
         ($fmt:expr, $($args:tt)*) => ({ /* compiler built-in */ })
@@ -1235,42 +1233,36 @@ pub(crate) mod builtin {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[allow_internal_unstable(test, rustc_attrs)]
     #[rustc_builtin_macro]
-    #[rustc_macro_transparency = "semitransparent"]
     pub macro test($item:item) { /* compiler built-in */ }
 
     /// Attribute macro applied to a function to turn it into a benchmark test.
     #[unstable(feature = "test", issue = "50297",
                reason = "`bench` is a part of custom test frameworks which are unstable")]
     #[allow_internal_unstable(test, rustc_attrs)]
     #[rustc_builtin_macro]
-    #[rustc_macro_transparency = "semitransparent"]
     pub macro bench($item:item) { /* compiler built-in */ }
 
     /// An implementation detail of the `#[test]` and `#[bench]` macros.
     #[unstable(feature = "custom_test_frameworks", issue = "50297",
                reason = "custom test frameworks are an unstable feature")]
     #[allow_internal_unstable(test, rustc_attrs)]
     #[rustc_builtin_macro]
-    #[rustc_macro_transparency = "semitransparent"]
     pub macro test_case($item:item) { /* compiler built-in */ }
 
     /// Attribute macro applied to a static to register it as a global allocator.
     #[stable(feature = "global_allocator", since = "1.28.0")]
     #[allow_internal_unstable(rustc_attrs)]
     #[rustc_builtin_macro]
-    #[rustc_macro_transparency = "semitransparent"]
     pub macro global_allocator($item:item) { /* compiler built-in */ }
 
     /// Unstable implementation detail of the `rustc` compiler, do not use.
     #[rustc_builtin_macro]
-    #[cfg_attr(boostrap_stdarch_ignore_this, rustc_macro_transparency = "semitransparent")]
     #[stable(feature = "rust1", since = "1.0.0")]
     #[allow_internal_unstable(core_intrinsics, libstd_sys_internals)]
     pub macro RustcDecodable($item:item) { /* compiler built-in */ }
 
     /// Unstable implementation detail of the `rustc` compiler, do not use.
     #[rustc_builtin_macro]
-    #[cfg_attr(boostrap_stdarch_ignore_this, rustc_macro_transparency = "semitransparent")]
     #[stable(feature = "rust1", since = "1.0.0")]
     #[allow_internal_unstable(core_intrinsics)]
     pub macro RustcEncodable($item:item) { /* compiler built-in */ }

src/librustc/ich/impls_syntax.rs

@@ -402,7 +402,6 @@ impl_stable_hash_for!(struct ::syntax_pos::hygiene::ExpnData {
     parent -> _,
     call_site,
     def_site,
-    default_transparency,
     allow_internal_unstable,
     allow_internal_unsafe,
     local_inner_macros,

src/librustc/ty/query/on_disk_cache.rs

@@ -23,7 +23,7 @@ use std::mem;
 use syntax::ast::NodeId;
 use syntax::source_map::{SourceMap, StableSourceFileId};
 use syntax_pos::{BytePos, Span, DUMMY_SP, SourceFile};
-use syntax_pos::hygiene::{ExpnId, SyntaxContext, ExpnData};
+use syntax_pos::hygiene::{ExpnId, SyntaxContext};
 
 const TAG_FILE_FOOTER: u128 = 0xC0FFEE_C0FFEE_C0FFEE_C0FFEE_C0FFEE;
 
@@ -593,8 +593,8 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for CacheDecoder<'a, 'tcx> {
         // don't seem to be used after HIR lowering, so everything should be fine
         // as long as incremental compilation does not kick in before that.
         let location = || Span::with_root_ctxt(lo, hi);
-        let recover_from_expn_data = |this: &Self, expn_data, pos| {
-            let span = location().fresh_expansion(expn_data);
+        let recover_from_expn_data = |this: &Self, expn_data, transparency, pos| {
+            let span = location().fresh_expansion_with_transparency(expn_data, transparency);
             this.synthetic_syntax_contexts.borrow_mut().insert(pos, span.ctxt());
             span
         };
@@ -603,9 +603,9 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for CacheDecoder<'a, 'tcx> {
                 location()
             }
             TAG_EXPN_DATA_INLINE => {
-                let expn_data = Decodable::decode(self)?;
+                let (expn_data, transparency) = Decodable::decode(self)?;
                 recover_from_expn_data(
-                    self, expn_data, AbsoluteBytePos::new(self.opaque.position())
+                    self, expn_data, transparency, AbsoluteBytePos::new(self.opaque.position())
                 )
             }
             TAG_EXPN_DATA_SHORTHAND => {
@@ -614,9 +614,9 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for CacheDecoder<'a, 'tcx> {
                 if let Some(ctxt) = cached_ctxt {
                     Span::new(lo, hi, ctxt)
                 } else {
-                    let expn_data =
-                        self.with_position(pos.to_usize(), |this| ExpnData::decode(this))?;
-                    recover_from_expn_data(self, expn_data, pos)
+                    let (expn_data, transparency) =
+                        self.with_position(pos.to_usize(), |this| Decodable::decode(this))?;
+                    recover_from_expn_data(self, expn_data, transparency, pos)
                 }
             }
             _ => {
@@ -819,15 +819,15 @@ where
         if span_data.ctxt == SyntaxContext::root() {
             TAG_NO_EXPN_DATA.encode(self)
         } else {
-            let (expn_id, expn_data) = span_data.ctxt.outer_expn_with_data();
+            let (expn_id, transparency, expn_data) = span_data.ctxt.outer_mark_with_data();
             if let Some(pos) = self.expn_data_shorthands.get(&expn_id).cloned() {
                 TAG_EXPN_DATA_SHORTHAND.encode(self)?;
                 pos.encode(self)
             } else {
                 TAG_EXPN_DATA_INLINE.encode(self)?;
                 let pos = AbsoluteBytePos::new(self.position());
                 self.expn_data_shorthands.insert(expn_id, pos);
-                expn_data.encode(self)
+                (expn_data, transparency).encode(self)
             }
         }
     }

src/librustc_resolve/build_reduced_graph.rs

@@ -145,7 +145,7 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    fn get_macro_by_def_id(&mut self, def_id: DefId) -> Option<Lrc<SyntaxExtension>> {
+    crate fn get_macro_by_def_id(&mut self, def_id: DefId) -> Option<Lrc<SyntaxExtension>> {
         if let Some(ext) = self.macro_map.get(&def_id) {
             return Some(ext.clone());
         }

src/librustc_resolve/lib.rs

@@ -1647,10 +1647,14 @@ impl<'a> Resolver<'a> {
             if module.expansion != parent.expansion &&
             module.expansion.is_descendant_of(parent.expansion) {
                 // The macro is a proc macro derive
-                if module.expansion.looks_like_proc_macro_derive() {
-                    if parent.expansion.outer_expn_is_descendant_of(span.ctxt()) {
-                        *poisoned = Some(node_id);
-                        return module.parent;
+                if let Some(&def_id) = self.macro_defs.get(&module.expansion) {
+                    if let Some(ext) = self.get_macro_by_def_id(def_id) {
+                        if !ext.is_builtin && ext.macro_kind() == MacroKind::Derive {
+                            if parent.expansion.outer_expn_is_descendant_of(span.ctxt()) {
+                                *poisoned = Some(node_id);
+                                return module.parent;
+                            }
+                        }
                     }
                 }
             }

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;
@@ -549,8 +549,6 @@ pub struct SyntaxExtension {
     pub kind: SyntaxExtensionKind,
     /// Span of the macro definition.
     pub span: Span,
-    /// Hygienic properties of spans produced by this macro by default.
-    pub default_transparency: Transparency,
     /// Whitelist of unstable features that are treated as stable inside this macro.
     pub allow_internal_unstable: Option<Lrc<[Symbol]>>,
     /// Suppresses the `unsafe_code` lint for code produced by this macro.
@@ -572,22 +570,6 @@ pub struct SyntaxExtension {
     pub is_derive_copy: bool,
 }
 
-impl SyntaxExtensionKind {
-    /// When a syntax extension is constructed,
-    /// its transparency can often be inferred from its kind.
-    fn default_transparency(&self) -> Transparency {
-        match self {
-            SyntaxExtensionKind::Bang(..) |
-            SyntaxExtensionKind::Attr(..) |
-            SyntaxExtensionKind::Derive(..) |
-            SyntaxExtensionKind::NonMacroAttr { .. } => Transparency::Opaque,
-            SyntaxExtensionKind::LegacyBang(..) |
-            SyntaxExtensionKind::LegacyAttr(..) |
-            SyntaxExtensionKind::LegacyDerive(..) => Transparency::SemiTransparent,
-        }
-    }
-}
-
 impl SyntaxExtension {
     /// Returns which kind of macro calls this syntax extension.
     pub fn macro_kind(&self) -> MacroKind {
@@ -606,7 +588,6 @@ impl SyntaxExtension {
     pub fn default(kind: SyntaxExtensionKind, edition: Edition) -> SyntaxExtension {
         SyntaxExtension {
             span: DUMMY_SP,
-            default_transparency: kind.default_transparency(),
             allow_internal_unstable: None,
             allow_internal_unsafe: false,
             local_inner_macros: false,
@@ -646,7 +627,6 @@ impl SyntaxExtension {
             parent,
             call_site,
             def_site: self.span,
-            default_transparency: self.default_transparency,
             allow_internal_unstable: self.allow_internal_unstable.clone(),
             allow_internal_unsafe: self.allow_internal_unsafe,
             local_inner_macros: self.local_inner_macros,
@@ -760,23 +740,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 +861,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()

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)
                 }
@@ -1389,17 +1388,3 @@ impl<'feat> ExpansionConfig<'feat> {
         self.features.map_or(false, |features| features.custom_inner_attributes)
     }
 }
-
-// A Marker adds the given mark to the syntax context.
-#[derive(Debug)]
-pub struct Marker(pub ExpnId);
-
-impl MutVisitor for Marker {
-    fn visit_span(&mut self, span: &mut Span) {
-        *span = span.apply_mark(self.0)
-    }
-
-    fn visit_mac(&mut self, mac: &mut ast::Mac) {
-        noop_visit_mac(mac, self)
-    }
-}

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

src/libsyntax/ext/tt/macro_rules.rs

@@ -19,6 +19,7 @@ use crate::{ast, attr, attr::TransparencyError};
 
 use errors::{DiagnosticBuilder, FatalError};
 use log::debug;
+use syntax_pos::hygiene::Transparency;
 use syntax_pos::Span;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -128,6 +129,7 @@ impl<'a> ParserAnyMacro<'a> {
 struct MacroRulesMacroExpander {
     name: ast::Ident,
     span: Span,
+    transparency: Transparency,
     lhses: Vec<quoted::TokenTree>,
     rhses: Vec<quoted::TokenTree>,
     valid: bool,
@@ -143,7 +145,9 @@ impl TTMacroExpander for MacroRulesMacroExpander {
         if !self.valid {
             return DummyResult::any(sp);
         }
-        generic_extension(cx, sp, self.span, self.name, input, &self.lhses, &self.rhses)
+        generic_extension(
+            cx, sp, self.span, self.name, self.transparency, input, &self.lhses, &self.rhses
+        )
     }
 }
 
@@ -158,6 +162,7 @@ fn generic_extension<'cx>(
     sp: Span,
     def_span: Span,
     name: ast::Ident,
+    transparency: Transparency,
     arg: TokenStream,
     lhses: &[quoted::TokenTree],
     rhses: &[quoted::TokenTree],
@@ -187,7 +192,7 @@ fn generic_extension<'cx>(
 
                 let rhs_spans = rhs.iter().map(|t| t.span()).collect::<Vec<_>>();
                 // rhs has holes ( `$id` and `$(...)` that need filled)
-                let mut tts = transcribe(cx, &named_matches, rhs);
+                let mut tts = transcribe(cx, &named_matches, rhs, transparency);
 
                 // Replace all the tokens for the corresponding positions in the macro, to maintain
                 // proper positions in error reporting, while maintaining the macro_backtrace.
@@ -415,11 +420,7 @@ pub fn compile(
     // that is not lint-checked and trigger the "failed to process buffered lint here" bug.
     valid &= macro_check::check_meta_variables(sess, ast::CRATE_NODE_ID, def.span, &lhses, &rhses);
 
-    let expander: Box<_> =
-        Box::new(MacroRulesMacroExpander { name: def.ident, span: def.span, lhses, rhses, valid });
-
-    let (default_transparency, transparency_error) =
-        attr::find_transparency(&def.attrs, body.legacy);
+    let (transparency, transparency_error) = attr::find_transparency(&def.attrs, body.legacy);
     match transparency_error {
         Some(TransparencyError::UnknownTransparency(value, span)) =>
             sess.span_diagnostic.span_err(
@@ -432,6 +433,10 @@ pub fn compile(
         None => {}
     }
 
+    let expander: Box<_> = Box::new(MacroRulesMacroExpander {
+        name: def.ident, span: def.span, transparency, lhses, rhses, valid
+    });
+
     let allow_internal_unstable =
         attr::find_by_name(&def.attrs, sym::allow_internal_unstable).map(|attr| {
             attr.meta_item_list()
@@ -473,7 +478,6 @@ pub fn compile(
     SyntaxExtension {
         kind: SyntaxExtensionKind::LegacyBang(expander),
         span: def.span,
-        default_transparency,
         allow_internal_unstable,
         allow_internal_unsafe: attr::contains_name(&def.attrs, sym::allow_internal_unsafe),
         local_inner_macros,