Auto merge of #60903 - nnethercote:mv-gensyms-from-Symbol-to-Ident, r=<try>

bors · bors · commit 139b5dab14ae · 2019-05-19T23:10:55.000Z
Move gensym operations from `Symbol` to `Ident` Gensyms are always at the `Ident` level, and long-term we probably want to record gensym-ness in hygiene data. r? @petrochenkov
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -856,7 +856,7 @@ impl<'a> LoweringContext<'a> {
     }
 
     fn str_to_ident(&self, s: &'static str) -> Ident {
-        Ident::with_empty_ctxt(Symbol::gensym(s))
+        Ident::from_str(s).gensym()
     }
 
     fn with_anonymous_lifetime_mode<R>(
diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs
@@ -140,7 +140,7 @@ impl MutVisitor for ExpandAllocatorDirectives<'_> {
 
         // Generate the submodule itself
         let name = f.kind.fn_name("allocator_abi");
-        let allocator_abi = Ident::with_empty_ctxt(Symbol::gensym(&name));
+        let allocator_abi = Ident::from_str(&name).gensym();
         let module = f.cx.item_mod(span, span, allocator_abi, Vec::new(), items);
         let module = f.cx.monotonic_expander().flat_map_item(module).pop().unwrap();
 
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -314,7 +314,7 @@ impl<'a> Resolver<'a> {
                             Ident::new(keywords::SelfLower.name(), new_span)
                         ),
                         kind: ast::UseTreeKind::Simple(
-                            Some(Ident::new(Name::gensym("__dummy"), new_span)),
+                            Some(Ident::from_str_and_span("__dummy", new_span).gensym()),
                             ast::DUMMY_NODE_ID,
                             ast::DUMMY_NODE_ID,
                         ),
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -4225,7 +4225,7 @@ impl<'a> Resolver<'a> {
         let add_module_candidates = |module: Module<'_>, names: &mut Vec<TypoSuggestion>| {
             for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
                 if let Some(binding) = resolution.borrow().binding {
-                    if !ident.name.is_gensymed() && filter_fn(binding.res()) {
+                    if !ident.is_gensymed() && filter_fn(binding.res()) {
                         names.push(TypoSuggestion {
                             candidate: ident.name,
                             article: binding.res().article(),
@@ -4243,7 +4243,7 @@ impl<'a> Resolver<'a> {
             for rib in self.ribs[ns].iter().rev() {
                 // Locals and type parameters
                 for (ident, &res) in &rib.bindings {
-                    if !ident.name.is_gensymed() && filter_fn(res) {
+                    if !ident.is_gensymed() && filter_fn(res) {
                         names.push(TypoSuggestion {
                             candidate: ident.name,
                             article: res.article(),
@@ -4273,7 +4273,7 @@ impl<'a> Resolver<'a> {
                                             },
                                         );
 
-                                        if !ident.name.is_gensymed() && filter_fn(crate_mod) {
+                                        if !ident.is_gensymed() && filter_fn(crate_mod) {
                                             Some(TypoSuggestion {
                                                 candidate: ident.name,
                                                 article: "a",
@@ -4298,7 +4298,6 @@ impl<'a> Resolver<'a> {
                 names.extend(
                     self.primitive_type_table.primitive_types
                         .iter()
-                        .filter(|(name, _)| !name.is_gensymed())
                         .map(|(name, _)| {
                             TypoSuggestion {
                                 candidate: *name,
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -1395,7 +1395,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
             // so they can cause name conflict errors downstream.
             let is_good_import = binding.is_import() && !binding.is_ambiguity() &&
                                  // Note that as_str() de-gensyms the Symbol
-                                 !(ident.name.is_gensymed() && ident.name.as_str() != "_");
+                                 !(ident.is_gensymed() && ident.name.as_str() != "_");
             if is_good_import || binding.is_macro_def() {
                 let res = binding.res();
                 if res != Res::Err {
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -72,11 +72,7 @@ pub struct Path {
 impl PartialEq<Symbol> for Path {
     fn eq(&self, symbol: &Symbol) -> bool {
         self.segments.len() == 1 && {
-            let name = self.segments[0].ident.name;
-            // Make sure these symbols are pure strings
-            debug_assert!(!symbol.is_gensymed());
-            debug_assert!(!name.is_gensymed());
-            name == *symbol
+            self.segments[0].ident.name == *symbol
         }
     }
 }
diff --git a/src/libsyntax/diagnostics/plugin.rs b/src/libsyntax/diagnostics/plugin.rs
@@ -7,7 +7,7 @@ use crate::ext::base::{ExtCtxt, MacEager, MacResult};
 use crate::ext::build::AstBuilder;
 use crate::parse::token;
 use crate::ptr::P;
-use crate::symbol::{keywords, Symbol};
+use crate::symbol::keywords;
 use crate::tokenstream::{TokenTree};
 
 use smallvec::smallvec;
@@ -121,13 +121,13 @@ pub fn expand_register_diagnostic<'cx>(ecx: &'cx mut ExtCtxt<'_>,
 
     let span = span.apply_mark(ecx.current_expansion.mark);
 
-    let sym = Ident::new(Symbol::gensym(&format!("__register_diagnostic_{}", code)), span);
+    let name = Ident::from_str_and_span(&format!("__register_diagnostic_{}", code), span).gensym();
 
     MacEager::items(smallvec![
         ecx.item_mod(
             span,
             span,
-            sym,
+            name,
             vec![],
             vec![],
         )
diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs
@@ -252,8 +252,8 @@ pub fn compile(
     def: &ast::Item,
     edition: Edition
 ) -> SyntaxExtension {
-    let lhs_nm = ast::Ident::with_empty_ctxt(Symbol::gensym("lhs"));
-    let rhs_nm = ast::Ident::with_empty_ctxt(Symbol::gensym("rhs"));
+    let lhs_nm = ast::Ident::from_str("lhs").gensym();
+    let rhs_nm = ast::Ident::from_str("rhs").gensym();
 
     // Parse the macro_rules! invocation
     let body = match def.node {
diff --git a/src/libsyntax/std_inject.rs b/src/libsyntax/std_inject.rs
@@ -2,7 +2,7 @@ use crate::ast;
 use crate::attr;
 use crate::edition::Edition;
 use crate::ext::hygiene::{Mark, SyntaxContext};
-use crate::symbol::{Symbol, keywords, sym};
+use crate::symbol::{Ident, Symbol, keywords, sym};
 use crate::source_map::{ExpnInfo, MacroAttribute, dummy_spanned, hygiene, respan};
 use crate::ptr::P;
 use crate::tokenstream::TokenStream;
@@ -63,18 +63,15 @@ pub fn maybe_inject_crates_ref(
 
     // .rev() to preserve ordering above in combination with insert(0, ...)
     let alt_std_name = alt_std_name.map(Symbol::intern);
-    for orig_name in names.iter().rev() {
-        let orig_name = Symbol::intern(orig_name);
-        let mut rename = orig_name;
+    for orig_name_str in names.iter().rev() {
         // HACK(eddyb) gensym the injected crates on the Rust 2018 edition,
         // so they don't accidentally interfere with the new import paths.
-        if rust_2018 {
-            rename = orig_name.gensymed();
-        }
-        let orig_name = if rename != orig_name {
-            Some(orig_name)
+        let orig_name_sym = Symbol::intern(orig_name_str);
+        let orig_name_ident = Ident::with_empty_ctxt(orig_name_sym);
+        let (rename, orig_name) = if rust_2018 {
+            (orig_name_ident.gensym(), Some(orig_name_sym))
         } else {
-            None
+            (orig_name_ident, None)
         };
         krate.module.items.insert(0, P(ast::Item {
             attrs: vec![attr::mk_attr_outer(
@@ -84,7 +81,7 @@ pub fn maybe_inject_crates_ref(
             )],
             vis: dummy_spanned(ast::VisibilityKind::Inherited),
             node: ast::ItemKind::ExternCrate(alt_std_name.or(orig_name)),
-            ident: ast::Ident::with_empty_ctxt(rename),
+            ident: rename,
             id: ast::DUMMY_NODE_ID,
             span: DUMMY_SP,
             tokens: None,
diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs
@@ -232,11 +232,11 @@ fn mk_reexport_mod(cx: &mut TestCtxt<'_>,
         items,
     };
 
-    let sym = Ident::with_empty_ctxt(Symbol::gensym("__test_reexports"));
+    let name = Ident::from_str("__test_reexports").gensym();
     let parent = if parent == ast::DUMMY_NODE_ID { ast::CRATE_NODE_ID } else { parent };
     cx.ext_cx.current_expansion.mark = cx.ext_cx.resolver.get_module_scope(parent);
     let it = cx.ext_cx.monotonic_expander().flat_map_item(P(ast::Item {
-        ident: sym,
+        ident: name,
         attrs: Vec::new(),
         id: ast::DUMMY_NODE_ID,
         node: ast::ItemKind::Mod(reexport_mod),
@@ -245,7 +245,7 @@ fn mk_reexport_mod(cx: &mut TestCtxt<'_>,
         tokens: None,
     })).pop().unwrap();
 
-    (it, sym)
+    (it, name)
 }
 
 /// Crawl over the crate, inserting test reexports and the test main function
@@ -373,9 +373,10 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P<ast::Item> {
                            main_body);
 
     // Honor the reexport_test_harness_main attribute
-    let main_id = Ident::new(
-        cx.reexport_test_harness_main.unwrap_or(Symbol::gensym("main")),
-        sp);
+    let main_id = match cx.reexport_test_harness_main {
+        Some(sym) => Ident::new(sym, sp),
+        None => Ident::from_str_and_span("main", sp).gensym(),
+    };
 
     P(ast::Item {
         ident: main_id,
diff --git a/src/libsyntax_ext/proc_macro_decls.rs b/src/libsyntax_ext/proc_macro_decls.rs
@@ -429,7 +429,7 @@ fn mk_decls(
     let module = cx.item_mod(
         span,
         span,
-        ast::Ident::with_empty_ctxt(Symbol::gensym("decls")),
+        ast::Ident::from_str("decls").gensym(),
         vec![doc_hidden],
         vec![krate, decls_static],
     ).map(|mut i| {
diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs
@@ -127,7 +127,7 @@ pub fn expand_test_or_bench(
         ])
     };
 
-    let mut test_const = cx.item(sp, ast::Ident::new(item.ident.name.gensymed(), sp),
+    let mut test_const = cx.item(sp, ast::Ident::new(item.ident.name, sp).gensym(),
         vec![
             // #[cfg(test)]
             cx.attribute(attr_sp, cx.meta_list(attr_sp, Symbol::intern("cfg"), vec![
diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs
@@ -623,10 +623,12 @@ pub struct Ident {
 
 impl Ident {
     #[inline]
+    /// constructs a new identifier from a symbol and a span.
     pub const fn new(name: Symbol, span: Span) -> Ident {
         Ident { name, span }
     }
 
+    /// Constructs a new identifier with an empty syntax context.
     #[inline]
     pub const fn with_empty_ctxt(name: Symbol) -> Ident {
         Ident::new(name, DUMMY_SP)
@@ -637,11 +639,16 @@ impl Ident {
         Ident::with_empty_ctxt(string.as_symbol())
     }
 
-    /// Maps a string to an identifier with an empty syntax context.
+    /// Maps a string to an identifier with an empty span.
     pub fn from_str(string: &str) -> Ident {
         Ident::with_empty_ctxt(Symbol::intern(string))
     }
 
+    /// Maps a string and a span to an identifier.
+    pub fn from_str_and_span(string: &str, span: Span) -> Ident {
+        Ident::new(Symbol::intern(string), span)
+    }
+
     /// Replaces `lo` and `hi` with those from `span`, but keep hygiene context.
     pub fn with_span_pos(self, span: Span) -> Ident {
         Ident::new(self.name, span.with_ctxt(self.span.ctxt()))
@@ -669,14 +676,23 @@ impl Ident {
         Ident::new(self.name, self.span.modern_and_legacy())
     }
 
+    /// Transforms an identifier into one with the same name, but gensymed.
     pub fn gensym(self) -> Ident {
-        Ident::new(self.name.gensymed(), self.span)
+        let name = with_interner(|interner| interner.gensymed(self.name));
+        Ident::new(name, self.span)
     }
 
+    /// 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 self.name == keywords::Underscore.name() { self.gensym() } else { self }
     }
 
+    // WARNING: this function is deprecated and will be removed in the future.
+    pub fn is_gensymed(self) -> bool {
+        with_interner(|interner| interner.is_gensymed(self.name))
+    }
+
     pub fn as_str(self) -> LocalInternedString {
         self.name.as_str()
     }
@@ -729,30 +745,34 @@ impl Decodable for Ident {
         Ok(if !string.starts_with('#') {
             Ident::from_str(&string)
         } else { // FIXME(jseyfried): intercrate hygiene
-            Ident::with_empty_ctxt(Symbol::gensym(&string[1..]))
+            Ident::from_str(&string[1..]).gensym()
         })
     }
 }
 
 /// A symbol is an interned or gensymed string. A gensym is a symbol that is
-/// never equal to any other symbol. E.g.:
-/// ```
-/// assert_eq!(Symbol::intern("x"), Symbol::intern("x"))
-/// assert_ne!(Symbol::gensym("x"), Symbol::intern("x"))
-/// assert_ne!(Symbol::gensym("x"), Symbol::gensym("x"))
-/// ```
+/// 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.
+/// 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.)
 ///
-/// Internally, a Symbol is implemented as an index, and all operations
+/// Examples:
+/// ```
+/// assert_eq!(Ident::from_str("x"), Ident::from_str("x"))
+/// assert_ne!(Ident::from_str("x").gensym(), Ident::from_str("x"))
+/// assert_ne!(Ident::from_str("x").gensym(), Ident::from_str("x").gensym())
+/// ```
+/// 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.
 ///
-/// Note that `Symbol` cannot directly be a `newtype_index!` because it implements
-/// `fmt::Debug`, `Encodable`, and `Decodable` in special ways.
+/// Note that `Symbol` cannot directly be a `newtype_index!` because it
+/// implements `fmt::Debug`, `Encodable`, and `Decodable` in special ways.
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Symbol(SymbolIndex);
 
@@ -770,20 +790,6 @@ impl Symbol {
         with_interner(|interner| interner.intern(string))
     }
 
-    /// Gensyms a new `usize`, using the current interner.
-    pub fn gensym(string: &str) -> Self {
-        with_interner(|interner| interner.gensym(string))
-    }
-
-    pub fn gensymed(self) -> Self {
-        with_interner(|interner| interner.gensymed(self))
-    }
-
-    // WARNING: this function is deprecated and will be removed in the future.
-    pub fn is_gensymed(self) -> bool {
-        with_interner(|interner| interner.is_gensymed(self))
-    }
-
     pub fn as_str(self) -> LocalInternedString {
         with_interner(|interner| unsafe {
             LocalInternedString {
@@ -891,11 +897,6 @@ impl Interner {
         }
     }
 
-    fn gensym(&mut self, string: &str) -> Symbol {
-        let symbol = self.intern(string);
-        self.gensymed(symbol)
-    }
-
     fn gensymed(&mut self, symbol: Symbol) -> Symbol {
         self.gensyms.push(symbol);
         Symbol::new(SymbolIndex::MAX_AS_U32 - self.gensyms.len() as u32 + 1)
@@ -1263,11 +1264,13 @@ mod tests {
         assert_eq!(i.intern("cat"), Symbol::new(1));
         // dog is still at zero
         assert_eq!(i.intern("dog"), Symbol::new(0));
-        assert_eq!(i.gensym("zebra"), Symbol::new(SymbolIndex::MAX_AS_U32));
+        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.gensym("zebra"), Symbol::new(SymbolIndex::MAX_AS_U32 - 1));
+        assert_eq!(i.gensymed(z), Symbol::new(SymbolIndex::MAX_AS_U32 - 1));
         // gensym of *existing* string gets new number:
-        assert_eq!(i.gensym("dog"), Symbol::new(SymbolIndex::MAX_AS_U32 - 2));
+        let d = i.intern("dog");
+        assert_eq!(i.gensymed(d), Symbol::new(SymbolIndex::MAX_AS_U32 - 2));
     }
 
     #[test]

Original file line number	Diff line number	Diff line change
`@@ -856,7 +856,7 @@ impl<'a> LoweringContext<'a> {`
`856`	`856`	`}`
`857`	`857`
`858`	`858`	`fn str_to_ident(&self, s: &'static str) -> Ident {`
`859`		`- Ident::with_empty_ctxt(Symbol::gensym(s))`
	`859`	`+ Ident::from_str(s).gensym()`
`860`	`860`	`}`
`861`	`861`
`862`	`862`	`fn with_anonymous_lifetime_mode<R>(`
Original file line number	Diff line number	Diff line change
`@@ -72,11 +72,7 @@ pub struct Path {`
`72`	`72`	`impl PartialEq<Symbol> for Path {`
`73`	`73`	`fn eq(&self, symbol: &Symbol) -> bool {`
`74`	`74`	`self.segments.len() == 1 && {`
`75`		`- let name = self.segments[0].ident.name;`
`76`		`- // Make sure these symbols are pure strings`
`77`		`- debug_assert!(!symbol.is_gensymed());`
`78`		`- debug_assert!(!name.is_gensymed());`
`79`		`- name == *symbol`
	`75`	`+ self.segments[0].ident.name == *symbol`
`80`	`76`	`}`
`81`	`77`	`}`
`82`	`78`	`}`