From 66153d76e25834812d2785675c98e95d5ecb1830 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 3 Dec 2021 07:12:31 +1100 Subject: [PATCH] Improve the comments in `Symbol::interner`. --- compiler/rustc_span/src/symbol.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 247d69d6ee9..dd6ce60abfb 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1716,8 +1716,9 @@ pub(crate) struct Interner(Lock); // found that to regress performance up to 2% in some cases. This might be // revisited after further improvements to `indexmap`. // -// This type is private to prevent accidentally constructing more than one `Interner` on the same -// thread, which makes it easy to mixup `Symbol`s between `Interner`s. +// This type is private to prevent accidentally constructing more than one +// `Interner` on the same thread, which makes it easy to mixup `Symbol`s +// between `Interner`s. #[derive(Default)] struct InternerInner { arena: DroplessArena, @@ -1743,14 +1744,20 @@ impl Interner { let name = Symbol::new(inner.strings.len() as u32); - // `from_utf8_unchecked` is safe since we just allocated a `&str` which is known to be - // UTF-8. + // SAFETY: we convert from `&str` to `&[u8]`, clone it into the arena, + // and immediately convert the clone back to `&[u8], all because there + // is no `inner.arena.alloc_str()` method. This is clearly safe. let string: &str = unsafe { str::from_utf8_unchecked(inner.arena.alloc_slice(string.as_bytes())) }; - // It is safe to extend the arena allocation to `'static` because we only access - // these while the arena is still alive. + + // SAFETY: we can extend the arena allocation to `'static` because we + // only access these while the arena is still alive. let string: &'static str = unsafe { &*(string as *const str) }; inner.strings.push(string); + + // This second hash table lookup can be avoided by using `RawEntryMut`, + // but this code path isn't hot enough for it to be worth it. See + // #91445 for details. inner.names.insert(string, name); name }