Split and rename Interner::get.

Most of the callers cannot be a gensym, so this commit introduces a new
function that doesn't look for that case. (It will panic with a bounds
check failure if that case somehow arises.)

The commit also adds comments better explaining `Symbol`,
`InternedString` and `LocalInternedString`.

Author: nnethercote

src/librustc_codegen_llvm/debuginfo/metadata.rs

@@ -170,7 +170,7 @@ impl TypeMap<'ll, 'tcx> {
     // the id is unknown.
     fn get_unique_type_id_as_string(&self, unique_type_id: UniqueTypeId) -> &str {
         let UniqueTypeId(interner_key) = unique_type_id;
-        self.unique_id_interner.get(interner_key)
+        self.unique_id_interner.symbol_str(interner_key)
     }
 
     // Get the UniqueTypeId for the given type. If the UniqueTypeId for the given
@@ -226,7 +226,7 @@ impl TypeMap<'ll, 'tcx> {
         let variant_part_type_id = format!("{}_variant_part",
                                            self.get_unique_type_id_as_string(enum_type_id));
         let interner_key = self.unique_id_interner.intern(&variant_part_type_id);
-        self.unique_id_interner.get(interner_key)
+        self.unique_id_interner.symbol_str(interner_key)
     }
 }
 

src/libsyntax_pos/symbol.rs

@@ -344,9 +344,18 @@ impl Decodable for Ident {
     }
 }
 
-/// A symbol is an interned or gensymed string. 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.
+/// A symbol is an interned or gensymed string. (A gensym is a special kind of
+/// symbol that is never equal to any other symbol. E.g.:
+/// - `Symbol::intern("x") == Symbol::intern("x")`
+/// - `Symbol::gensym("x") != Symbol::intern("x")`
+/// - `Symbol::gensym("x") != Symbol::gensym("x")`
+///
+/// Gensyms are useful when creating new identifiers that must not match any
+/// existing identifiers during macro expansion and syntax desugaring.)
+///
+/// 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.
@@ -387,7 +396,7 @@ impl Symbol {
     pub fn as_str(self) -> LocalInternedString {
         with_interner(|interner| unsafe {
             LocalInternedString {
-                string: std::mem::transmute::<&str, &str>(interner.get(self))
+                string: std::mem::transmute::<&str, &str>(interner.symbol_or_gensym_str(self))
             }
         })
     }
@@ -510,7 +519,13 @@ impl Interner {
         symbol.0.as_usize() >= self.strings.len()
     }
 
-    pub fn get(&self, symbol: Symbol) -> &str {
+    /// Get the chars of a normal (non-gensym) symbol.
+    pub fn symbol_str(&self, symbol: Symbol) -> &str {
+        self.strings[symbol.0.as_usize()]
+    }
+
+    /// Get the chars of a normal or gensym symbol.
+    fn symbol_or_gensym_str(&self, symbol: Symbol) -> &str {
         match self.strings.get(symbol.0.as_usize()) {
             Some(string) => string,
             None => {
@@ -614,11 +629,17 @@ fn with_interner<T, F: FnOnce(&mut Interner) -> T>(f: F) -> T {
     GLOBALS.with(|globals| f(&mut *globals.symbol_interner.lock()))
 }
 
-/// Represents a string stored in the interner. Because the interner outlives any thread
-/// which uses this type, we can safely treat `string` which points to interner data,
-/// as an immortal string, as long as this type never crosses between threads.
-// FIXME: ensure that the interner outlives any thread which uses `LocalInternedString`,
-// by creating a new thread right after constructing the interner.
+/// An alternative to `Symbol` and `LocalInternedString`, useful when the chars
+/// within the symbol need to be accessed. It is best used for temporary
+/// values.
+///
+/// Because the interner outlives any thread which uses this type, we can
+/// safely treat `string` which points to interner data, as an immortal string,
+/// as long as this type never crosses between threads.
+//
+// FIXME: ensure that the interner outlives any thread which uses
+// `LocalInternedString`, by creating a new thread right after constructing the
+// interner.
 #[derive(Clone, Copy, Hash, PartialOrd, Eq, Ord)]
 pub struct LocalInternedString {
     string: &'static str,
@@ -711,7 +732,12 @@ impl Encodable for LocalInternedString {
     }
 }
 
-/// Represents a string stored in the string interner.
+/// A thin wrapper around `Symbol`. It has two main differences to `Symbol`.
+/// - 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.
+/// - It is guaranteed to not be a gensym symbol.
 #[derive(Clone, Copy, Eq)]
 pub struct InternedString {
     symbol: Symbol,
@@ -720,7 +746,7 @@ pub struct InternedString {
 impl InternedString {
     pub fn with<F: FnOnce(&str) -> R, R>(self, f: F) -> R {
         let str = with_interner(|interner| {
-            interner.get(self.symbol) as *const str
+            interner.symbol_str(self.symbol) as *const str
         });
         // This is safe because the interner keeps string alive until it is dropped.
         // We can access it because we know the interner is still alive since we use a
@@ -730,8 +756,8 @@ impl InternedString {
 
     pub fn with2<F: FnOnce(&str, &str) -> R, R>(self, other: &InternedString, f: F) -> R {
         let (self_str, other_str) = with_interner(|interner| {
-            (interner.get(self.symbol) as *const str,
-             interner.get(other.symbol) as *const str)
+            (interner.symbol_str(self.symbol) as *const str,
+             interner.symbol_str(other.symbol) as *const str)
         });
         // This is safe for the same reason that `with` is safe.
         unsafe { f(&*self_str, &*other_str) }