From f6637f3fcc01d7efe9a2e00f62d35a7e68ae892d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 May 2019 09:35:26 +1000 Subject: [PATCH 1/3] Move `is_gensymed` from `Symbol` to `Ident`. Note that the `is_gensymed` call on `primitive_types` is unnecessary because that table only contains the name of primitive types (e.g. `i32`) and never contains gensyms. --- src/librustc_resolve/lib.rs | 7 +++---- src/librustc_resolve/resolve_imports.rs | 2 +- src/libsyntax/ast.rs | 6 +----- src/libsyntax_pos/symbol.rs | 10 +++++----- 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 77e8cc3272cc3..c4a4dd306055b 100644 --- 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| { 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 index 4058f0bce0f95..3a6a8b56ff35d 100644 --- 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 index d12240655e628..88c22085bc4d2 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -72,11 +72,7 @@ pub struct Path { impl PartialEq 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_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 97b22282668ad..3cd5577e19e49 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -684,6 +684,11 @@ impl 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() } @@ -786,11 +791,6 @@ impl Symbol { 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 { From e57c0dbeb72cf631930429a20badd41290bb0608 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 May 2019 10:27:17 +1000 Subject: [PATCH 2/3] Eliminate `Symbol::gensymed`. --- src/libsyntax/std_inject.rs | 13 ++++--------- src/libsyntax_ext/test.rs | 2 +- src/libsyntax_pos/symbol.rs | 7 ++----- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/libsyntax/std_inject.rs b/src/libsyntax/std_inject.rs index 6784a2abe551c..fe8c9f03a2c6b 100644 --- a/src/libsyntax/std_inject.rs +++ b/src/libsyntax/std_inject.rs @@ -63,18 +63,13 @@ 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 (rename, orig_name) = if rust_2018 { + (Symbol::gensym(orig_name_str), Some(Symbol::intern(orig_name_str))) } else { - None + (Symbol::intern(orig_name_str), None) }; krate.module.items.insert(0, P(ast::Item { attrs: vec![attr::mk_attr_outer( diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs index 86ae6ab5fece5..211a098022f98 100644 --- 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 index 3cd5577e19e49..2bf3b1fa4657d 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -677,7 +677,8 @@ impl Ident { } 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) } pub fn gensym_if_underscore(self) -> Ident { @@ -787,10 +788,6 @@ impl Symbol { with_interner(|interner| interner.gensym(string)) } - pub fn gensymed(self) -> Self { - with_interner(|interner| interner.gensymed(self)) - } - pub fn as_str(self) -> LocalInternedString { with_interner(|interner| unsafe { LocalInternedString { From 88d29992bdd3d881b336b724675c59323e70e2e7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 May 2019 10:44:51 +1000 Subject: [PATCH 3/3] Remove `Symbol::gensym()`. --- src/librustc_allocator/expand.rs | 2 +- src/librustc_resolve/build_reduced_graph.rs | 2 +- src/libsyntax/diagnostics/plugin.rs | 6 +-- src/libsyntax/ext/tt/macro_rules.rs | 4 +- src/libsyntax/std_inject.rs | 10 ++-- src/libsyntax/test.rs | 13 ++--- src/libsyntax_ext/proc_macro_decls.rs | 2 +- src/libsyntax_pos/symbol.rs | 56 ++++++++++++--------- 8 files changed, 52 insertions(+), 43 deletions(-) diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs index 585c6fde63450..6931b3542f761 100644 --- 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 index f70ca6f859b98..3b58a99d19fe0 100644 --- 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/libsyntax/diagnostics/plugin.rs b/src/libsyntax/diagnostics/plugin.rs index 21024eb41ef50..c988dc61bec44 100644 --- 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 index 06651750de741..672b7b4285522 100644 --- 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 index fe8c9f03a2c6b..1be7986ad53cd 100644 --- 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; @@ -66,10 +66,12 @@ pub fn maybe_inject_crates_ref( 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. + 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 { - (Symbol::gensym(orig_name_str), Some(Symbol::intern(orig_name_str))) + (orig_name_ident.gensym(), Some(orig_name_sym)) } else { - (Symbol::intern(orig_name_str), None) + (orig_name_ident, None) }; krate.module.items.insert(0, P(ast::Item { attrs: vec![attr::mk_attr_outer( @@ -79,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 index 3fd0790161cdd..3dc7aad945939 100644 --- 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 { 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 index c582fb422c970..200445d124880 100644 --- 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_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 2bf3b1fa4657d..fb99be0fe33f3 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -630,10 +630,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) @@ -644,11 +646,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())) @@ -676,11 +683,14 @@ 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 { 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 } } @@ -742,30 +752,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` 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); @@ -783,11 +797,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 as_str(self) -> LocalInternedString { with_interner(|interner| unsafe { LocalInternedString { @@ -895,11 +904,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) @@ -1267,11 +1271,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]