Auto merge of #64623 - matthewjasper:underscore-imports, r=<try>

Remove last uses of gensyms

Bindings are now indexed in resolve with an additional disambiguator that's used for underscore bindings.  This is the last use of gensyms in the compiler.

I'm not completely happy with this approach, so suggestions are welcome. Moving undescore bindings into their own map didn't turn out any better: master...matthewjasper:remove-underscore-gensyms.

closes #49300
cc #60869

r? @petrochenkov

src/librustc/ich/impls_syntax.rs

@@ -373,6 +373,7 @@ impl_stable_hash_for!(enum ::syntax_pos::hygiene::ExpnKind {
     Root,
     Macro(kind, descr),
     AstPass(kind),
+    Underscore,
     Desugaring(kind)
 });
 

src/librustc/lint/mod.rs

@@ -871,7 +871,9 @@ pub fn provide(providers: &mut Providers<'_>) {
 pub fn in_external_macro(sess: &Session, span: Span) -> bool {
     let expn_data = span.ctxt().outer_expn_data();
     match expn_data.kind {
-        ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop) => false,
+        ExpnKind::Root
+        | ExpnKind::Underscore
+        | ExpnKind::Desugaring(DesugaringKind::ForLoop) => false,
         ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
         ExpnKind::Macro(MacroKind::Bang, _) => {
             if expn_data.def_site.is_dummy() {

src/librustc/ty/print/pretty.rs

@@ -1480,7 +1480,7 @@ impl<F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
         }
 
         // Replace any anonymous late-bound regions with named
-        // variants, using gensym'd identifiers, so that we can
+        // variants, using new unique identifiers, so that we can
         // clearly differentiate between named and unnamed regions in
         // the output. We'll probably want to tweak this over time to
         // decide just how much information to give.

src/librustc_resolve/build_reduced_graph.rs

@@ -406,7 +406,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         };
         match use_tree.kind {
             ast::UseTreeKind::Simple(rename, ..) => {
-                let mut ident = use_tree.ident().gensym_if_underscore();
+                let mut ident = use_tree.ident().unique_if_underscore();
                 let mut module_path = prefix;
                 let mut source = module_path.pop().unwrap();
                 let mut type_ns_only = false;
@@ -584,7 +584,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         let parent_scope = &self.parent_scope;
         let parent = parent_scope.module;
         let expansion = parent_scope.expansion;
-        let ident = item.ident.gensym_if_underscore();
+        let ident = item.ident.unique_if_underscore();
         let sp = item.span;
         let vis = self.resolve_visibility(&item.vis);
 
@@ -849,10 +849,10 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
     fn build_reduced_graph_for_external_crate_res(&mut self, child: Export<NodeId>) {
         let parent = self.parent_scope.module;
         let Export { ident, res, vis, span } = child;
-        // FIXME: We shouldn't create the gensym here, it should come from metadata,
-        // but metadata cannot encode gensyms currently, so we create it here.
-        // This is only a guess, two equivalent idents may incorrectly get different gensyms here.
-        let ident = ident.gensym_if_underscore();
+        // FIXME: We shouldn't create the SyntaxContext here, it should come from metadata,
+        // but metadata doesn't encode hygiene information currently, so we create it here.
+        // This is only a guess, two equivalent idents will get different SyntaxContexts here.
+        let ident = ident.unique_if_underscore();
         let expansion = ExpnId::root(); // FIXME(jseyfried) intercrate hygiene
         // Record primary definitions.
         match res {

src/librustc_resolve/resolve_imports.rs

@@ -1340,7 +1340,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
             // hygiene.
             // FIXME: Implement actual cross-crate hygiene.
             let is_good_import = binding.is_import() && !binding.is_ambiguity()
-                && !ident.span.modern().from_expansion();
+                && (!ident.span.from_expansion() || ident.name == kw::Underscore);
             if is_good_import || binding.is_macro_def() {
                 let res = binding.res();
                 if res != Res::Err {
@@ -1350,8 +1350,8 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                         }
                     }
                     reexports.push(Export {
-                        ident: ident.modern(),
-                        res: res,
+                        ident,
+                        res,
                         span: binding.span,
                         vis: binding.vis,
                     });
@@ -1418,6 +1418,10 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         }
 
         if reexports.len() > 0 {
+            // Type pretty printing will prefer reexports that appear earlier
+            // in this map, so sort them to ensure that diagnostic output is
+            // stable.
+            reexports.sort_by_cached_key(|export| export.ident.as_str());
             if let Some(def_id) = module.def_id() {
                 self.r.export_map.insert(def_id, reexports);
             }

src/libsyntax/ext/proc_macro_server.rs

@@ -332,8 +332,7 @@ impl Ident {
         if !Self::is_valid(&string) {
             panic!("`{:?}` is not a valid identifier", string)
         }
-        // Get rid of gensyms to conservatively check rawness on the string contents only.
-        if is_raw && !sym.as_interned_str().as_symbol().can_be_raw() {
+        if is_raw && !sym.can_be_raw() {
             panic!("`{}` cannot be a raw identifier", string);
         }
         Ident { sym, is_raw, span }

src/libsyntax_pos/hygiene.rs

@@ -217,8 +217,14 @@ impl HygieneData {
 
     fn adjust(&self, ctxt: &mut SyntaxContext, expn_id: ExpnId) -> Option<ExpnId> {
         let mut scope = None;
-        while !self.is_descendant_of(expn_id, self.outer_expn(*ctxt)) {
-            scope = Some(self.remove_mark(ctxt).0);
+        let mut outer = self.outer_expn(*ctxt);
+        while !self.is_descendant_of(expn_id, outer) {
+            if let ExpnData { kind: ExpnKind::Underscore, call_site, .. } = *self.expn_data(outer) {
+                *ctxt = call_site.ctxt();
+            } else {
+                scope = Some(self.remove_mark(ctxt).0);
+            }
+            outer = self.outer_expn(*ctxt);
         }
         scope
     }
@@ -420,6 +426,21 @@ impl SyntaxContext {
         HygieneData::with(|data| data.marks(self))
     }
 
+    crate fn unique(base_span: Span) -> Self {
+        HygieneData::with(|data| {
+            // We store the original span as the call-site so that we can
+            // recover it. We call `modern` here to save us calling it when we
+            // access `call_site`.
+            let modern = base_span.with_ctxt(data.modern(base_span.ctxt()));
+            let expn_id = data.fresh_expn(Some(ExpnData::default(
+                ExpnKind::Underscore,
+                modern,
+                data.expn_data(data.outer_expn(modern.ctxt())).edition,
+            )));
+            data.apply_mark(SyntaxContext::root(), expn_id, Transparency::Opaque)
+        })
+    }
+
     /// Adjust this context for resolution in a scope created by the given expansion.
     /// For example, consider the following three resolutions of `f`:
     ///
@@ -508,7 +529,7 @@ impl SyntaxContext {
     pub fn reverse_glob_adjust(&mut self, expn_id: ExpnId, glob_span: Span)
                                -> Option<Option<ExpnId>> {
         HygieneData::with(|data| {
-            if data.adjust(self, expn_id).is_some() {
+            if data.adjust(&mut {*self}, expn_id).is_some() {
                 return None;
             }
 
@@ -674,6 +695,8 @@ pub enum ExpnKind {
     Macro(MacroKind, Symbol),
     /// Transform done by the compiler on the AST.
     AstPass(AstPass),
+    /// Generated by name resolution to give underscores unique identifiers.
+    Underscore,
     /// Desugaring done by the compiler during HIR lowering.
     Desugaring(DesugaringKind)
 }
@@ -684,6 +707,7 @@ impl ExpnKind {
             ExpnKind::Root => kw::PathRoot,
             ExpnKind::Macro(_, descr) => descr,
             ExpnKind::AstPass(kind) => Symbol::intern(kind.descr()),
+            ExpnKind::Underscore => kw::Underscore,
             ExpnKind::Desugaring(kind) => Symbol::intern(kind.descr()),
         }
     }

src/libsyntax_pos/lib.rs

@@ -443,6 +443,7 @@ impl Span {
                     ExpnKind::Root => break,
                     ExpnKind::Desugaring(..) => ("desugaring of ", ""),
                     ExpnKind::AstPass(..) => ("", ""),
+                    ExpnKind::Underscore => ("", ""),
                     ExpnKind::Macro(macro_kind, _) => match macro_kind {
                         MacroKind::Bang => ("", "!"),
                         MacroKind::Attr => ("#[", "]"),

src/libsyntax_pos/symbol.rs

@@ -15,7 +15,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::str;
 
-use crate::{Span, DUMMY_SP, GLOBALS};
+use crate::{Span, SyntaxContext, DUMMY_SP, GLOBALS};
 
 #[cfg(test)]
 mod tests;
@@ -803,15 +803,14 @@ impl Ident {
         Ident::new(self.name, self.span.modern_and_legacy())
     }
 
-    /// Transforms an underscore identifier into one with the same name, but
-    /// gensymed. Leaves non-underscore identifiers unchanged.
-    pub fn gensym_if_underscore(self) -> Ident {
+    /// If the name of this ident is "_", then return a new unique identifier.
+    /// Otherwise returns `self` unmodified
+    #[inline]
+    pub fn unique_if_underscore(mut self) -> Ident {
         if self.name == kw::Underscore {
-            let name = with_interner(|interner| interner.gensymed(self.name));
-            Ident::new(name, self.span)
-        } else {
-            self
+            self.span = self.span.with_ctxt(SyntaxContext::unique(self.span));
         }
+        self
     }
 
     /// Convert the name to a `LocalInternedString`. This is a slowish
@@ -820,8 +819,7 @@ impl Ident {
         self.name.as_str()
     }
 
-    /// Convert the name to an `InternedString`. This is a slowish operation
-    /// because it requires locking the symbol interner.
+    /// Convert the name to an `InternedString`.
     pub fn as_interned_str(self) -> InternedString {
         self.name.as_interned_str()
     }
@@ -876,26 +874,9 @@ impl UseSpecializedDecodable for Ident {
     }
 }
 
-/// A symbol is an interned or gensymed string. A gensym is a symbol that is
-/// never equal to any other symbol.
-///
-/// Conceptually, a gensym can be thought of as a normal symbol with an
-/// invisible unique suffix. Gensyms are useful when creating new identifiers
-/// that must not match any existing identifiers, e.g. during macro expansion
-/// and syntax desugaring. Because gensyms should always be identifiers, all
-/// gensym operations are on `Ident` rather than `Symbol`. (Indeed, in the
-/// future the gensym-ness may be moved from `Symbol` to hygiene data.)
+/// An interned string.
 ///
-/// Examples:
-/// ```
-/// assert_eq!(Ident::from_str("_"), Ident::from_str("_"))
-/// assert_ne!(Ident::from_str("_").gensym_if_underscore(), Ident::from_str("_"))
-/// assert_ne!(
-///     Ident::from_str("_").gensym_if_underscore(),
-///     Ident::from_str("_").gensym_if_underscore(),
-/// )
-/// ```
-/// Internally, a symbol is implemented as an index, and all operations
+/// Internally, a `Symbol` is implemented as an index, and all operations
 /// (including hashing, equality, and ordering) operate on that index. The use
 /// of `newtype_index!` means that `Option<Symbol>` only takes up 4 bytes,
 /// because `newtype_index!` reserves the last 256 values for tagging purposes.
@@ -946,12 +927,9 @@ impl Symbol {
         })
     }
 
-    /// Convert to an `InternedString`. This is a slowish operation because it
-    /// requires locking the symbol interner.
+    /// Convert to an `InternedString`.
     pub fn as_interned_str(self) -> InternedString {
-        with_interner(|interner| InternedString {
-            symbol: interner.interned(self)
-        })
+        InternedString { symbol: self }
     }
 
     pub fn as_u32(self) -> u32 {
@@ -961,12 +939,7 @@ impl Symbol {
 
 impl fmt::Debug for Symbol {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let is_gensymed = with_interner(|interner| interner.is_gensymed(*self));
-        if is_gensymed {
-            write!(f, "{}({:?})", self, self.0)
-        } else {
-            write!(f, "{}", self)
-        }
+        fmt::Display::fmt(self, f)
     }
 }
 
@@ -989,15 +962,11 @@ impl Decodable for Symbol {
 }
 
 // The `&'static str`s in this type actually point into the arena.
-//
-// Note that normal symbols are indexed upward from 0, and gensyms are indexed
-// downward from SymbolIndex::MAX_AS_U32.
 #[derive(Default)]
 pub struct Interner {
     arena: DroplessArena,
     names: FxHashMap<&'static str, Symbol>,
     strings: Vec<&'static str>,
-    gensyms: Vec<Symbol>,
 }
 
 impl Interner {
@@ -1030,34 +999,10 @@ impl Interner {
         self.names.insert(string, name);
         name
     }
-
-    fn interned(&self, symbol: Symbol) -> Symbol {
-        if (symbol.0.as_usize()) < self.strings.len() {
-            symbol
-        } else {
-            self.gensyms[(SymbolIndex::MAX_AS_U32 - symbol.0.as_u32()) as usize]
-        }
-    }
-
-    fn gensymed(&mut self, symbol: Symbol) -> Symbol {
-        self.gensyms.push(symbol);
-        Symbol::new(SymbolIndex::MAX_AS_U32 - self.gensyms.len() as u32 + 1)
-    }
-
-    fn is_gensymed(&mut self, symbol: Symbol) -> bool {
-        symbol.0.as_usize() >= self.strings.len()
-    }
-
     // Get the symbol as a string. `Symbol::as_str()` should be used in
     // preference to this function.
     pub fn get(&self, symbol: Symbol) -> &str {
-        match self.strings.get(symbol.0.as_usize()) {
-            Some(string) => string,
-            None => {
-                let symbol = self.gensyms[(SymbolIndex::MAX_AS_U32 - symbol.0.as_u32()) as usize];
-                self.strings[symbol.0.as_usize()]
-            }
-        }
+        self.strings[symbol.0.as_usize()]
     }
 }
 
@@ -1242,19 +1187,12 @@ impl fmt::Display for LocalInternedString {
     }
 }
 
-/// An alternative to `Symbol` that is focused on string contents. It has two
-/// main differences to `Symbol`.
+/// An alternative to `Symbol` that is focused on string contents.
 ///
-/// First, its implementations of `Hash`, `PartialOrd` and `Ord` work with the
+/// Its implementations of `Hash`, `PartialOrd` and `Ord` work with the
 /// string chars rather than the symbol integer. This is useful when hash
 /// stability is required across compile sessions, or a guaranteed sort
 /// ordering is required.
-///
-/// Second, gensym-ness is irrelevant. E.g.:
-/// ```
-/// assert_ne!(Symbol::gensym("x"), Symbol::gensym("x"))
-/// assert_eq!(Symbol::gensym("x").as_interned_str(), Symbol::gensym("x").as_interned_str())
-/// ```
 #[derive(Clone, Copy, PartialEq, Eq)]
 pub struct InternedString {
     symbol: Symbol,

src/libsyntax_pos/symbol/tests.rs

@@ -14,13 +14,6 @@ fn interner_tests() {
     assert_eq!(i.intern("cat"), Symbol::new(1));
     // dog is still at zero
     assert_eq!(i.intern("dog"), Symbol::new(0));
-    let z = i.intern("zebra");
-    assert_eq!(i.gensymed(z), Symbol::new(SymbolIndex::MAX_AS_U32));
-    // gensym of same string gets new number:
-    assert_eq!(i.gensymed(z), Symbol::new(SymbolIndex::MAX_AS_U32 - 1));
-    // gensym of *existing* string gets new number:
-    let d = i.intern("dog");
-    assert_eq!(i.gensymed(d), Symbol::new(SymbolIndex::MAX_AS_U32 - 2));
 }
 
 #[test]

src/test/ui/underscore-imports/hygiene-2.rs

@@ -0,0 +1,33 @@
+// Make sure that underscore imports with different contexts can exist in the
+// same scope.
+
+// check-pass
+
+#![feature(decl_macro)]
+
+mod x {
+    pub use std::ops::Deref as _;
+}
+
+macro n() {
+    pub use crate::x::*;
+}
+
+#[macro_export]
+macro_rules! p {
+    () => { pub use crate::x::*; }
+}
+
+macro m($y:ident) {
+    mod $y {
+        crate::n!(); // Reexport of `Deref` should not be imported in `main`
+        crate::p!(); // Reexport of `Deref` should be imported into `main`
+    }
+}
+
+m!(y);
+
+fn main() {
+    use crate::y::*;
+    (&()).deref();
+}