Rollup merge of #64141 - nnethercote:minimize-LocalInternedString, r=petrochenkov

Minimize uses of `LocalInternedString`

`LocalInternedString` is described as "An alternative to `Symbol` and `InternedString`, useful when the chars within the symbol need to be accessed. It is best used for temporary values."

This PR makes the code match that comment, by removing all non-local uses of `LocalInternedString`. This allows the removal of a number of operations on `LocalInternedString` and a couple of uses of `unsafe`.
This commit is contained in:
Mazdak Farrokhzad 2019-09-05 03:59:49 +02:00 committed by GitHub
commit 4dfbaba3bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 86 additions and 133 deletions

View File

@ -186,7 +186,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
});
let mut upstream_crates: Vec<_> = cstore.crates_untracked().iter().map(|&cnum| {
let name = cstore.crate_name_untracked(cnum).as_str();
let name = cstore.crate_name_untracked(cnum).as_interned_str();
let disambiguator = cstore.crate_disambiguator_untracked(cnum).to_fingerprint();
let hash = cstore.crate_hash_untracked(cnum);
(name, disambiguator, hash)

View File

@ -9,7 +9,7 @@ use std::mem;
use syntax::ast;
use syntax::feature_gate;
use syntax::parse::token;
use syntax::symbol::{InternedString, LocalInternedString};
use syntax::symbol::InternedString;
use syntax::tokenstream;
use syntax_pos::SourceFile;
@ -39,27 +39,6 @@ impl<'a> ToStableHashKey<StableHashingContext<'a>> for InternedString {
}
}
impl<'a> HashStable<StableHashingContext<'a>> for LocalInternedString {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
let s: &str = &**self;
s.hash_stable(hcx, hasher);
}
}
impl<'a> ToStableHashKey<StableHashingContext<'a>> for LocalInternedString {
type KeyType = LocalInternedString;
#[inline]
fn to_stable_hash_key(&self,
_: &StableHashingContext<'a>)
-> LocalInternedString {
self.clone()
}
}
impl<'a> HashStable<StableHashingContext<'a>> for ast::Name {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,

View File

@ -33,7 +33,7 @@ use crate::util::common::time;
use std::default::Default as StdDefault;
use syntax::ast;
use syntax::edition;
use syntax_pos::{MultiSpan, Span, symbol::{LocalInternedString, Symbol}};
use syntax_pos::{MultiSpan, Span, symbol::Symbol};
use errors::DiagnosticBuilder;
use crate::hir;
use crate::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
@ -405,7 +405,7 @@ impl LintStore {
pub fn check_lint_name(
&self,
lint_name: &str,
tool_name: Option<LocalInternedString>,
tool_name: Option<Symbol>,
) -> CheckLintNameResult<'_> {
let complete_name = if let Some(tool_name) = tool_name {
format!("{}::{}", tool_name, lint_name)

View File

@ -291,7 +291,7 @@ impl<'a> LintLevelsBuilder<'a> {
continue;
}
Some(tool_ident.as_str())
Some(tool_ident.name)
} else {
None
};

View File

@ -9,10 +9,9 @@ use syntax::ast::{MetaItem, NestedMetaItem};
use syntax::attr;
use syntax::symbol::{Symbol, kw, sym};
use syntax_pos::Span;
use syntax_pos::symbol::LocalInternedString;
#[derive(Clone, Debug)]
pub struct OnUnimplementedFormatString(LocalInternedString);
pub struct OnUnimplementedFormatString(Symbol);
#[derive(Debug)]
pub struct OnUnimplementedDirective {
@ -89,19 +88,19 @@ impl<'tcx> OnUnimplementedDirective {
if item.check_name(sym::message) && message.is_none() {
if let Some(message_) = item.value_str() {
message = Some(OnUnimplementedFormatString::try_parse(
tcx, trait_def_id, message_.as_str(), span)?);
tcx, trait_def_id, message_, span)?);
continue;
}
} else if item.check_name(sym::label) && label.is_none() {
if let Some(label_) = item.value_str() {
label = Some(OnUnimplementedFormatString::try_parse(
tcx, trait_def_id, label_.as_str(), span)?);
tcx, trait_def_id, label_, span)?);
continue;
}
} else if item.check_name(sym::note) && note.is_none() {
if let Some(note_) = item.value_str() {
note = Some(OnUnimplementedFormatString::try_parse(
tcx, trait_def_id, note_.as_str(), span)?);
tcx, trait_def_id, note_, span)?);
continue;
}
} else if item.check_name(sym::on) && is_root &&
@ -154,7 +153,7 @@ impl<'tcx> OnUnimplementedDirective {
message: None,
subcommands: vec![],
label: Some(OnUnimplementedFormatString::try_parse(
tcx, trait_def_id, value.as_str(), attr.span)?),
tcx, trait_def_id, value, attr.span)?),
note: None,
}))
} else {
@ -218,7 +217,7 @@ impl<'tcx> OnUnimplementedFormatString {
fn try_parse(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
from: LocalInternedString,
from: Symbol,
err_sp: Span,
) -> Result<Self, ErrorReported> {
let result = OnUnimplementedFormatString(from);
@ -234,7 +233,8 @@ impl<'tcx> OnUnimplementedFormatString {
) -> Result<(), ErrorReported> {
let name = tcx.item_name(trait_def_id);
let generics = tcx.generics_of(trait_def_id);
let parser = Parser::new(&self.0, None, vec![], false);
let s = self.0.as_str();
let parser = Parser::new(&s, None, vec![], false);
let mut result = Ok(());
for token in parser {
match token {
@ -294,7 +294,8 @@ impl<'tcx> OnUnimplementedFormatString {
}).collect::<FxHashMap<Symbol, String>>();
let empty_string = String::new();
let parser = Parser::new(&self.0, None, vec![], false);
let s = self.0.as_str();
let parser = Parser::new(&s, None, vec![], false);
parser.map(|p|
match p {
Piece::String(s) => s,

View File

@ -46,7 +46,7 @@ use std::ops::Range;
use syntax::ast::{self, Name, Ident, NodeId};
use syntax::attr;
use syntax::ext::hygiene::ExpnId;
use syntax::symbol::{kw, sym, Symbol, LocalInternedString, InternedString};
use syntax::symbol::{kw, sym, Symbol, InternedString};
use syntax_pos::Span;
use smallvec;
@ -3386,10 +3386,6 @@ impl SymbolName {
name: InternedString::intern(name)
}
}
pub fn as_str(&self) -> LocalInternedString {
self.name.as_str()
}
}
impl fmt::Display for SymbolName {

View File

@ -5,7 +5,6 @@ use crate::context::CodegenCx;
use crate::type_::Type;
use crate::type_of::LayoutLlvmExt;
use crate::value::Value;
use syntax::symbol::LocalInternedString;
use rustc_codegen_ssa::common::{IntPredicate, TypeKind, RealPredicate};
use rustc_codegen_ssa::MemFlags;
use libc::{c_uint, c_char};
@ -24,6 +23,7 @@ use std::ffi::CStr;
use std::ops::{Deref, Range};
use std::ptr;
use std::iter::TrustedLen;
use syntax::symbol::Symbol;
// All Builders must have an llfn associated with them
#[must_use]
@ -1082,8 +1082,8 @@ impl StaticBuilderMethods for Builder<'a, 'll, 'tcx> {
fn static_panic_msg(
&mut self,
msg: Option<LocalInternedString>,
filename: LocalInternedString,
msg: Option<Symbol>,
filename: Symbol,
line: Self::Value,
col: Self::Value,
kind: &str,

View File

@ -37,7 +37,7 @@ pub fn get_fn(
return llfn;
}
let sym = tcx.symbol_name(instance).as_str();
let sym = tcx.symbol_name(instance).name.as_str();
debug!("get_fn({:?}: {:?}) => {}", instance, sig, sym);
// Create a fn pointer with the substituted signature.

View File

@ -17,7 +17,7 @@ use rustc_codegen_ssa::mir::place::PlaceRef;
use libc::{c_uint, c_char};
use syntax::symbol::LocalInternedString;
use syntax::symbol::Symbol;
use syntax::ast::Mutability;
pub use crate::context::CodegenCx;
@ -122,7 +122,7 @@ impl CodegenCx<'ll, 'tcx> {
fn const_cstr(
&self,
s: LocalInternedString,
s: Symbol,
null_terminated: bool,
) -> &'ll Value {
unsafe {
@ -130,9 +130,10 @@ impl CodegenCx<'ll, 'tcx> {
return llval;
}
let s_str = s.as_str();
let sc = llvm::LLVMConstStringInContext(self.llcx,
s.as_ptr() as *const c_char,
s.len() as c_uint,
s_str.as_ptr() as *const c_char,
s_str.len() as c_uint,
!null_terminated as Bool);
let sym = self.generate_local_symbol_name("str");
let g = self.define_global(&sym[..], self.val_ty(sc)).unwrap_or_else(||{
@ -147,8 +148,8 @@ impl CodegenCx<'ll, 'tcx> {
}
}
pub fn const_str_slice(&self, s: LocalInternedString) -> &'ll Value {
let len = s.len();
pub fn const_str_slice(&self, s: Symbol) -> &'ll Value {
let len = s.as_str().len();
let cs = consts::ptrcast(self.const_cstr(s, false),
self.type_ptr_to(self.layout_of(self.tcx.mk_str()).llvm_type(self)));
self.const_fat_ptr(cs, self.const_usize(len as u64))

View File

@ -11,12 +11,11 @@ use rustc::mir::interpret::{ConstValue, Allocation, read_target_uint,
Pointer, ErrorHandled, GlobalId};
use rustc::mir::mono::MonoItem;
use rustc::hir::Node;
use syntax_pos::Span;
use rustc_target::abi::HasDataLayout;
use syntax::symbol::sym;
use syntax_pos::symbol::LocalInternedString;
use rustc::ty::{self, Ty, Instance};
use rustc_codegen_ssa::traits::*;
use syntax::symbol::{Symbol, sym};
use syntax_pos::Span;
use rustc::ty::layout::{self, Size, Align, LayoutOf};
@ -122,10 +121,11 @@ fn check_and_apply_linkage(
cx: &CodegenCx<'ll, 'tcx>,
attrs: &CodegenFnAttrs,
ty: Ty<'tcx>,
sym: LocalInternedString,
sym: Symbol,
span: Span
) -> &'ll Value {
let llty = cx.layout_of(ty).llvm_type(cx);
let sym = sym.as_str();
if let Some(linkage) = attrs.linkage {
debug!("get_static: sym={} linkage={:?}", sym, linkage);
@ -221,7 +221,7 @@ impl CodegenCx<'ll, 'tcx> {
def_id);
let ty = instance.ty(self.tcx);
let sym = self.tcx.symbol_name(instance).as_str();
let sym = self.tcx.symbol_name(instance).name.as_symbol();
debug!("get_static: sym={} instance={:?}", sym, instance);
@ -232,11 +232,12 @@ impl CodegenCx<'ll, 'tcx> {
Node::Item(&hir::Item {
ref attrs, span, node: hir::ItemKind::Static(..), ..
}) => {
if self.get_declared_value(&sym[..]).is_some() {
let sym_str = sym.as_str();
if self.get_declared_value(&sym_str).is_some() {
span_bug!(span, "Conflicting symbol names for static?");
}
let g = self.define_global(&sym[..], llty).unwrap();
let g = self.define_global(&sym_str, llty).unwrap();
if !self.tcx.is_reachable_non_generic(def_id) {
unsafe {

View File

@ -29,7 +29,7 @@ use std::cell::{Cell, RefCell};
use std::iter;
use std::str;
use std::sync::Arc;
use syntax::symbol::LocalInternedString;
use syntax::symbol::Symbol;
use syntax::source_map::{DUMMY_SP, Span};
use crate::abi::Abi;
@ -52,7 +52,7 @@ pub struct CodegenCx<'ll, 'tcx> {
pub vtables:
RefCell<FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), &'ll Value>>,
/// Cache of constant strings,
pub const_cstr_cache: RefCell<FxHashMap<LocalInternedString, &'ll Value>>,
pub const_cstr_cache: RefCell<FxHashMap<Symbol, &'ll Value>>,
/// Reverse-direction for const ptrs cast from globals.
/// Key is a Value holding a *T,

View File

@ -2251,7 +2251,7 @@ pub fn create_global_var_metadata(
None
} else {
let linkage_name = mangled_name_of_instance(cx, Instance::mono(tcx, def_id));
Some(SmallCStr::new(&linkage_name.as_str()))
Some(SmallCStr::new(&linkage_name.name.as_str()))
};
let global_align = cx.align_of(variable_type);

View File

@ -290,7 +290,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let scope_line = span_start(self, span).line;
let function_name = CString::new(name).unwrap();
let linkage_name = SmallCStr::new(&linkage_name.as_str());
let linkage_name = SmallCStr::new(&linkage_name.name.as_str());
let mut flags = DIFlags::FlagPrototyped;

View File

@ -121,7 +121,7 @@ fn reachable_non_generics_provider(
})
.map(|def_id| {
let export_level = if special_runtime_crate {
let name = tcx.symbol_name(Instance::mono(tcx, def_id)).as_str();
let name = tcx.symbol_name(Instance::mono(tcx, def_id)).name.as_str();
// We can probably do better here by just ensuring that
// it has hidden visibility rather than public
// visibility, as this is primarily here to ensure it's

View File

@ -14,7 +14,7 @@ use crate::traits::*;
use std::borrow::Cow;
use syntax::symbol::LocalInternedString;
use syntax::symbol::Symbol;
use syntax_pos::Pos;
use super::{FunctionCx, LocalRef};
@ -397,7 +397,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// Get the location information.
let loc = bx.sess().source_map().lookup_char_pos(span.lo());
let filename = LocalInternedString::intern(&loc.file.name.to_string());
let filename = Symbol::intern(&loc.file.name.to_string());
let line = bx.const_u32(loc.line as u32);
let col = bx.const_u32(loc.col.to_usize() as u32 + 1);
@ -418,8 +418,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
vec![file_line_col, index, len])
}
_ => {
let str = msg.description();
let msg_str = LocalInternedString::intern(str);
let msg_str = Symbol::intern(msg.description());
let msg_file_line_col = bx.static_panic_msg(
Some(msg_str),
filename,
@ -531,7 +530,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let layout = bx.layout_of(ty);
if layout.abi.is_uninhabited() {
let loc = bx.sess().source_map().lookup_char_pos(span.lo());
let filename = LocalInternedString::intern(&loc.file.name.to_string());
let filename = Symbol::intern(&loc.file.name.to_string());
let line = bx.const_u32(loc.line as u32);
let col = bx.const_u32(loc.col.to_usize() as u32 + 1);
@ -539,7 +538,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
"Attempted to instantiate uninhabited type {}",
ty
);
let msg_str = LocalInternedString::intern(&str);
let msg_str = Symbol::intern(&str);
let msg_file_line_col = bx.static_panic_msg(
Some(msg_str),
filename,

View File

@ -58,7 +58,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
self.to_raw_string(),
cx.codegen_unit().name());
let symbol_name = self.symbol_name(cx.tcx()).as_str();
let symbol_name = self.symbol_name(cx.tcx()).name.as_str();
debug!("symbol {}", &symbol_name);

View File

@ -1,5 +1,5 @@
use super::BackendTypes;
use syntax_pos::symbol::LocalInternedString;
use syntax_pos::symbol::Symbol;
use rustc::hir::def_id::DefId;
use rustc::ty::layout::Align;
@ -12,8 +12,8 @@ pub trait StaticBuilderMethods: BackendTypes {
fn get_static(&mut self, def_id: DefId) -> Self::Value;
fn static_panic_msg(
&mut self,
msg: Option<LocalInternedString>,
filename: LocalInternedString,
msg: Option<Symbol>,
filename: Symbol,
line: Self::Value,
col: Self::Value,
kind: &str,

View File

@ -40,7 +40,7 @@ impl SymbolNamesTest<'tcx> {
let instance = Instance::mono(tcx, def_id);
let mangled = self.tcx.symbol_name(instance);
tcx.sess.span_err(attr.span, &format!("symbol-name({})", mangled));
if let Ok(demangling) = rustc_demangle::try_demangle(&mangled.as_str()) {
if let Ok(demangling) = rustc_demangle::try_demangle(&mangled.name.as_str()) {
tcx.sess.span_err(attr.span, &format!("demangling({})", demangling));
tcx.sess.span_err(attr.span, &format!("demangling-alt({:#})", demangling));
}

View File

@ -336,7 +336,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let local = &self.body.local_decls[local_index];
match local.name {
Some(name) if !local.from_compiler_desugaring() => {
buf.push_str(name.as_str().get());
buf.push_str(&name.as_str());
Ok(())
}
_ => Err(()),

View File

@ -777,7 +777,7 @@ where
debug!("CodegenUnit {}:", cgu.name());
for (mono_item, linkage) in cgu.items() {
let symbol_name = mono_item.symbol_name(tcx).as_str();
let symbol_name = mono_item.symbol_name(tcx).name.as_str();
let symbol_hash_start = symbol_name.rfind('h');
let symbol_hash = symbol_hash_start.map(|i| &symbol_name[i ..])
.unwrap_or("<no hash>");

View File

@ -19,7 +19,7 @@ use crate::astconv::AstConv as _;
use errors::{Applicability, DiagnosticBuilder};
use syntax::ast;
use syntax::symbol::{Symbol, LocalInternedString, kw, sym};
use syntax::symbol::{Symbol, kw, sym};
use syntax::source_map::Span;
use syntax::util::lev_distance::find_best_match_for_name;
use rustc::hir;
@ -1244,7 +1244,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => {
// prevent all specified fields from being suggested
let skip_fields = skip_fields.iter().map(|ref x| x.ident.as_str());
let skip_fields = skip_fields.iter().map(|ref x| x.ident.name);
if let Some(field_name) = Self::suggest_field_name(
variant,
&field.ident.as_str(),
@ -1288,11 +1288,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Return an hint about the closest match in field names
fn suggest_field_name(variant: &'tcx ty::VariantDef,
field: &str,
skip: Vec<LocalInternedString>)
skip: Vec<Symbol>)
-> Option<Symbol> {
let names = variant.fields.iter().filter_map(|field| {
// ignore already set fields and private fields from non-local crates
if skip.iter().any(|x| *x == field.ident.as_str()) ||
if skip.iter().any(|&x| x == field.ident.name) ||
(!variant.def_id.is_local() && field.vis != Visibility::Public)
{
None

View File

@ -818,10 +818,14 @@ impl Ident {
with_interner(|interner| interner.is_gensymed(self.name))
}
/// Convert the name to a `LocalInternedString`. This is a slowish
/// operation because it requires locking the symbol interner.
pub fn as_str(self) -> LocalInternedString {
self.name.as_str()
}
/// Convert the name to an `InternedString`. This is a slowish operation
/// because it requires locking the symbol interner.
pub fn as_interned_str(self) -> InternedString {
self.name.as_interned_str()
}
@ -916,6 +920,25 @@ impl Symbol {
with_interner(|interner| interner.intern(string))
}
/// Access the symbol's chars. This is a slowish operation because it
/// requires locking the symbol interner.
pub fn with<F: FnOnce(&str) -> R, R>(self, f: F) -> R {
with_interner(|interner| {
f(interner.get(self))
})
}
/// Access two symbols' chars. This is a slowish operation because it
/// requires locking the symbol interner, but it is faster than calling
/// `with()` twice.
fn with2<F: FnOnce(&str, &str) -> R, R>(self, other: Symbol, f: F) -> R {
with_interner(|interner| {
f(interner.get(self), interner.get(other))
})
}
/// Convert to a `LocalInternedString`. This is a slowish operation because
/// it requires locking the symbol interner.
pub fn as_str(self) -> LocalInternedString {
with_interner(|interner| unsafe {
LocalInternedString {
@ -924,6 +947,8 @@ impl Symbol {
})
}
/// Convert to an `InternedString`. This is a slowish operation because it
/// requires locking the symbol interner.
pub fn as_interned_str(self) -> InternedString {
with_interner(|interner| InternedString {
symbol: interner.interned(self)
@ -1152,39 +1177,11 @@ fn with_interner<T, F: FnOnce(&mut Interner) -> T>(f: F) -> T {
// 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)]
#[derive(Clone, Copy, Eq, PartialOrd, Ord)]
pub struct LocalInternedString {
string: &'static str,
}
impl LocalInternedString {
/// Maps a string to its interned representation.
pub fn intern(string: &str) -> Self {
let string = with_interner(|interner| {
let symbol = interner.intern(string);
interner.strings[symbol.0.as_usize()]
});
LocalInternedString {
string: unsafe { std::mem::transmute::<&str, &str>(string) }
}
}
pub fn as_interned_str(self) -> InternedString {
InternedString {
symbol: Symbol::intern(self.string)
}
}
#[inline]
pub fn get(&self) -> &str {
// This returns a valid string since we ensure that `self` outlives the interner
// by creating the interner on a thread which outlives threads which can access it.
// This type cannot move to a thread which outlives the interner since it does
// not implement Send.
self.string
}
}
impl<U: ?Sized> std::convert::AsRef<U> for LocalInternedString
where
str: std::convert::AsRef<U>
@ -1246,18 +1243,6 @@ impl fmt::Display for LocalInternedString {
}
}
impl Decodable for LocalInternedString {
fn decode<D: Decoder>(d: &mut D) -> Result<LocalInternedString, D::Error> {
Ok(LocalInternedString::intern(&d.read_str()?))
}
}
impl Encodable for LocalInternedString {
fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
s.emit_str(self.string)
}
}
/// An alternative to `Symbol` that is focused on string contents. It has two
/// main differences to `Symbol`.
///
@ -1285,28 +1270,19 @@ 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
});
// 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
// scoped thread local to access it, and it was alive at the beginning of this scope
unsafe { f(&*str) }
self.symbol.with(f)
}
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)
});
// This is safe for the same reason that `with` is safe.
unsafe { f(&*self_str, &*other_str) }
self.symbol.with2(other.symbol, f)
}
pub fn as_symbol(self) -> Symbol {
self.symbol
}
/// Convert to a `LocalInternedString`. This is a slowish operation because it
/// requires locking the symbol interner.
pub fn as_str(self) -> LocalInternedString {
self.symbol.as_str()
}