Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change SymbolName::name to a &str. #74214

Merged
merged 2 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn get_fn(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> &'ll Value
return llfn;
}

let sym = tcx.symbol_name(instance).name.as_str();
let sym = tcx.symbol_name(instance).name;
debug!("get_fn({:?}: {:?}) => {}", instance, instance.monomorphic_ty(cx.tcx()), sym);

let fn_abi = FnAbi::of_instance(cx, instance, &[]);
Expand Down
10 changes: 4 additions & 6 deletions src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_middle::mir::interpret::{
use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::{sym, Symbol};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size};

Expand Down Expand Up @@ -107,11 +107,10 @@ fn check_and_apply_linkage(
cx: &CodegenCx<'ll, 'tcx>,
attrs: &CodegenFnAttrs,
ty: Ty<'tcx>,
sym: Symbol,
sym: &str,
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);

Expand Down Expand Up @@ -215,14 +214,13 @@ impl CodegenCx<'ll, 'tcx> {
// FIXME: refactor this to work without accessing the HIR
let (g, attrs) = match self.tcx.hir().get(id) {
Node::Item(&hir::Item { attrs, span, kind: hir::ItemKind::Static(..), .. }) => {
let sym_str = sym.as_str();
if let Some(g) = self.get_declared_value(&sym_str) {
if let Some(g) = self.get_declared_value(sym) {
if self.val_ty(g) != self.type_ptr_to(llty) {
span_bug!(span, "Conflicting types for static");
}
}

let g = self.declare_global(&sym_str, llty);
let g = self.declare_global(sym, llty);

if !self.tcx.is_reachable_non_generic(def_id) {
unsafe {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_codegen_llvm/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2468,8 +2468,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global
let variable_type = Instance::mono(cx.tcx, def_id).monomorphic_ty(cx.tcx);
let type_metadata = type_metadata(cx, variable_type, span);
let var_name = tcx.item_name(def_id).as_str();
let linkage_name: &str =
&mangled_name_of_instance(cx, Instance::mono(tcx, def_id)).name.as_str();
let linkage_name = mangled_name_of_instance(cx, Instance::mono(tcx, def_id)).name;
// When empty, linkage_name field is omitted,
// which is what we want for no_mangle statics
let linkage_name = if var_name == linkage_name { "" } else { linkage_name };
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let substs = instance.substs.truncate_to(self.tcx(), generics);
let template_parameters = get_template_parameters(self, &generics, substs, &mut name);

let linkage_name: &str = &mangled_name_of_instance(self, instance).name.as_str();
let linkage_name = &mangled_name_of_instance(self, instance).name;
// Omit the linkage_name if it is the same as subprogram name.
let linkage_name = if &name == linkage_name { "" } else { linkage_name };

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/debuginfo/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_hir::definitions::DefPathData;
pub fn mangled_name_of_instance<'a, 'tcx>(
cx: &CodegenCx<'a, 'tcx>,
instance: Instance<'tcx>,
) -> ty::SymbolName {
) -> ty::SymbolName<'tcx> {
let tcx = cx.tcx;
tcx.symbol_name(instance)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
// and/or monomorphization invalidates these assumptions.
let coverageinfo = tcx.coverageinfo(caller_instance.def_id());
let mangled_fn = tcx.symbol_name(caller_instance);
let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name);
let (mangled_fn_name, _len_val) = self.const_str(Symbol::intern(mangled_fn.name));
let hash = self.const_u64(coverageinfo.hash);
let num_counters = self.const_u32(coverageinfo.num_counters);
use coverage::count_code_region_args::*;
Expand Down
17 changes: 8 additions & 9 deletions src/librustc_codegen_ssa/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
use rustc_middle::ty::Instance;
use rustc_middle::ty::{SymbolName, TyCtxt};
use rustc_session::config::{CrateType, SanitizerSet};
use rustc_span::symbol::sym;

pub fn threshold(tcx: TyCtxt<'_>) -> SymbolExportLevel {
crates_export_threshold(&tcx.sess.crate_types())
Expand Down Expand Up @@ -117,9 +116,9 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, cnum: CrateNum) -> DefIdMap<
// In general though we won't link right if these
// symbols are stripped, and LTO currently strips them.
match name {
sym::rust_eh_personality
| sym::rust_eh_register_frames
| sym::rust_eh_unregister_frames =>
"rust_eh_personality"
| "rust_eh_register_frames"
| "rust_eh_unregister_frames" =>
SymbolExportLevel::C,
_ => SymbolExportLevel::Rust,
}
Expand Down Expand Up @@ -177,15 +176,15 @@ fn exported_symbols_provider_local(
.collect();

if tcx.entry_fn(LOCAL_CRATE).is_some() {
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new("main"));
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, "main"));

symbols.push((exported_symbol, SymbolExportLevel::C));
}

if tcx.allocator_kind().is_some() {
for method in ALLOCATOR_METHODS {
let symbol_name = format!("__rust_{}", method.name);
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(&symbol_name));
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name));

symbols.push((exported_symbol, SymbolExportLevel::Rust));
}
Expand All @@ -199,7 +198,7 @@ fn exported_symbols_provider_local(
["__llvm_profile_raw_version", "__llvm_profile_filename"];

symbols.extend(PROFILER_WEAK_SYMBOLS.iter().map(|sym| {
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(sym));
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, sym));
(exported_symbol, SymbolExportLevel::C)
}));
}
Expand All @@ -209,14 +208,14 @@ fn exported_symbols_provider_local(
const MSAN_WEAK_SYMBOLS: [&str; 2] = ["__msan_track_origins", "__msan_keep_going"];

symbols.extend(MSAN_WEAK_SYMBOLS.iter().map(|sym| {
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(sym));
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, sym));
(exported_symbol, SymbolExportLevel::C)
}));
}

if tcx.sess.crate_types().contains(&CrateType::Dylib) {
let symbol_name = metadata_symbol_name(tcx);
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(&symbol_name));
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name));

symbols.push((exported_symbol, SymbolExportLevel::Rust));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
cx.codegen_unit().name()
);

let symbol_name = self.symbol_name(cx.tcx()).name.as_str();
let symbol_name = self.symbol_name(cx.tcx()).name;

debug!("symbol {}", &symbol_name);

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ impl<'tcx> EncodeContext<'tcx> {
// Encode exported symbols info. This is prefetched in `encode_metadata` so we encode
// this late to give the prefetching as much time as possible to complete.
i = self.position();
let exported_symbols = self.tcx.exported_symbols(LOCAL_CRATE);
let exported_symbols = tcx.exported_symbols(LOCAL_CRATE);
let exported_symbols = self.encode_exported_symbols(&exported_symbols);
let exported_symbols_bytes = self.position() - i;

Expand Down Expand Up @@ -622,7 +622,7 @@ impl<'tcx> EncodeContext<'tcx> {

let total_bytes = self.position();

if self.tcx.sess.meta_stats() {
if tcx.sess.meta_stats() {
let mut zero_bytes = 0;
for e in self.opaque.data.iter() {
if *e == 0 {
Expand Down Expand Up @@ -1541,7 +1541,7 @@ impl EncodeContext<'tcx> {
) -> Lazy<[(ExportedSymbol<'tcx>, SymbolExportLevel)]> {
// The metadata symbol name is special. It should not show up in
// downstream crates.
let metadata_symbol_name = SymbolName::new(&metadata_symbol_name(self.tcx));
let metadata_symbol_name = SymbolName::new(self.tcx, &metadata_symbol_name(self.tcx));

self.lazy(
exported_symbols
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_middle/middle/exported_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ pub enum ExportedSymbol<'tcx> {
NonGeneric(DefId),
Generic(DefId, SubstsRef<'tcx>),
DropGlue(Ty<'tcx>),
NoDefId(ty::SymbolName),
NoDefId(ty::SymbolName<'tcx>),
}

impl<'tcx> ExportedSymbol<'tcx> {
/// This is the symbol name of an instance if it is instantiated in the
/// local crate.
pub fn symbol_name_for_local_instance(&self, tcx: TyCtxt<'tcx>) -> ty::SymbolName {
pub fn symbol_name_for_local_instance(&self, tcx: TyCtxt<'tcx>) -> ty::SymbolName<'tcx> {
match *self {
ExportedSymbol::NonGeneric(def_id) => tcx.symbol_name(ty::Instance::mono(tcx, def_id)),
ExportedSymbol::Generic(def_id, substs) => {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_middle/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ impl<'tcx> MonoItem<'tcx> {
}
}

pub fn symbol_name(&self, tcx: TyCtxt<'tcx>) -> SymbolName {
pub fn symbol_name(&self, tcx: TyCtxt<'tcx>) -> SymbolName<'tcx> {
match *self {
MonoItem::Fn(instance) => tcx.symbol_name(instance),
MonoItem::Static(def_id) => tcx.symbol_name(Instance::mono(tcx, def_id)),
MonoItem::GlobalAsm(hir_id) => {
let def_id = tcx.hir().local_def_id(hir_id);
SymbolName { name: Symbol::intern(&format!("global_asm_{:?}", def_id)) }
SymbolName::new(tcx, &format!("global_asm_{:?}", def_id))
}
}
}
Expand Down Expand Up @@ -335,9 +335,9 @@ impl<'tcx> CodegenUnit<'tcx> {
// The codegen tests rely on items being process in the same order as
// they appear in the file, so for local items, we sort by node_id first
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub struct ItemSortKey(Option<HirId>, SymbolName);
pub struct ItemSortKey<'tcx>(Option<HirId>, SymbolName<'tcx>);

fn item_sort_key<'tcx>(tcx: TyCtxt<'tcx>, item: MonoItem<'tcx>) -> ItemSortKey {
fn item_sort_key<'tcx>(tcx: TyCtxt<'tcx>, item: MonoItem<'tcx>) -> ItemSortKey<'tcx> {
ItemSortKey(
match item {
MonoItem::Fn(ref instance) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_middle/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ rustc_queries! {
/// The `symbol_name` query provides the symbol name for calling a
/// given instance from the local crate. In particular, it will also
/// look up the correct symbol name of instances from upstream crates.
query symbol_name(key: ty::Instance<'tcx>) -> ty::SymbolName {
query symbol_name(key: ty::Instance<'tcx>) -> ty::SymbolName<'tcx> {
desc { "computing the symbol for `{}`", key }
cache_on_disk_if { true }
}
Expand Down
15 changes: 15 additions & 0 deletions src/librustc_middle/ty/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ where
Ok(decoder.tcx().adt_def(def_id))
}

#[inline]
pub fn decode_symbol_name<D>(decoder: &mut D) -> Result<ty::SymbolName<'tcx>, D::Error>
where
D: TyDecoder<'tcx>,
{
Ok(ty::SymbolName::new(decoder.tcx(), &decoder.read_str()?))
}

#[inline]
pub fn decode_existential_predicate_slice<D>(
decoder: &mut D,
Expand Down Expand Up @@ -504,6 +512,13 @@ macro_rules! implement_ty_decoder {
}
}

impl<'_x, $($typaram),*> SpecializedDecoder<ty::SymbolName<'_x>>
for $DecoderName<$($typaram),*> {
fn specialized_decode(&mut self) -> Result<ty::SymbolName<'_x>, Self::Error> {
unsafe { transmute(decode_symbol_name(self)) }
}
}

impl<'_x, '_y, $($typaram),*> SpecializedDecoder<&'_x ty::List<ty::ExistentialPredicate<'_y>>>
for $DecoderName<$($typaram),*>
where &'_x ty::List<ty::ExistentialPredicate<'_y>>: UseSpecializedDecodable {
Expand Down
42 changes: 20 additions & 22 deletions src/librustc_middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ use std::hash::{Hash, Hasher};
use std::marker::PhantomData;
use std::ops::Range;
use std::ptr;
use std::str;

pub use self::sty::BoundRegion::*;
pub use self::sty::InferTy::*;
Expand Down Expand Up @@ -2988,40 +2989,37 @@ pub struct CrateInherentImpls {
pub inherent_impls: DefIdMap<Vec<DefId>>,
}

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)]
pub struct SymbolName {
// FIXME: we don't rely on interning or equality here - better have
// this be a `&'tcx str`.
pub name: Symbol,
}

impl SymbolName {
pub fn new(name: &str) -> SymbolName {
SymbolName { name: Symbol::intern(name) }
}
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, HashStable)]
pub struct SymbolName<'tcx> {
/// `&str` gives a consistent ordering, which ensures reproducible builds.
pub name: &'tcx str,
}

impl PartialOrd for SymbolName {
fn partial_cmp(&self, other: &SymbolName) -> Option<Ordering> {
self.name.as_str().partial_cmp(&other.name.as_str())
impl<'tcx> SymbolName<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, name: &str) -> SymbolName<'tcx> {
SymbolName {
name: unsafe { str::from_utf8_unchecked(tcx.arena.alloc_slice(name.as_bytes())) },
}
}
}

/// Ordering must use the chars to ensure reproducible builds.
impl Ord for SymbolName {
fn cmp(&self, other: &SymbolName) -> Ordering {
self.name.as_str().cmp(&other.name.as_str())
impl<'tcx> fmt::Display for SymbolName<'tcx> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.name, fmt)
}
}

impl fmt::Display for SymbolName {
impl<'tcx> fmt::Debug for SymbolName<'tcx> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.name, fmt)
}
}

impl fmt::Debug for SymbolName {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.name, fmt)
impl<'tcx> rustc_serialize::UseSpecializedEncodable for SymbolName<'tcx> {
fn default_encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
s.emit_str(self.name)
}
}

// The decoding takes place in `decode_symbol_name()`.
impl<'tcx> rustc_serialize::UseSpecializedDecodable for SymbolName<'tcx> {}
14 changes: 9 additions & 5 deletions src/librustc_middle/ty/query/values.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use crate::ty::{self, AdtSizedConstraint, Ty, TyCtxt, TyS};

use rustc_span::symbol::Symbol;

pub(super) trait Value<'tcx>: Sized {
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self;
}
Expand All @@ -21,9 +19,15 @@ impl<'tcx> Value<'tcx> for &'_ TyS<'_> {
}
}

impl<'tcx> Value<'tcx> for ty::SymbolName {
fn from_cycle_error(_: TyCtxt<'tcx>) -> Self {
ty::SymbolName { name: Symbol::intern("<error>") }
impl<'tcx> Value<'tcx> for ty::SymbolName<'_> {
fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
// SAFETY: This is never called when `Self` is not `SymbolName<'tcx>`.
// FIXME: Represent the above fact in the trait system somehow.
Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't you write ty::SymbolName<'tcx>? That's why Value has a 'tcx parameter.

Oh this is a specialization issue. IMO it should be noted as such rather then left as a vague FIXME (cc @matthewjasper).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly would you like me to write in the comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope @matthewjasper can come up with a better comment. Otherwise it's probably fine as-is.
Although I kind of doubt this from_cycle_error implementation is needed at all.

unsafe {
std::mem::transmute::<ty::SymbolName<'tcx>, ty::SymbolName<'_>>(ty::SymbolName::new(
tcx, "<error>",
))
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/monomorphize/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ where
debug!("CodegenUnit {} estimated size {} :", cgu.name(), cgu.size_estimate());

for (mono_item, linkage) in cgu.items() {
let symbol_name = mono_item.symbol_name(tcx).name.as_str();
let symbol_name = mono_item.symbol_name(tcx).name;
let symbol_hash_start = symbol_name.rfind('h');
let symbol_hash =
symbol_hash_start.map(|i| &symbol_name[i..]).unwrap_or("<no hash>");
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -856,8 +856,6 @@ symbols! {
rustc_unsafe_specialization_marker,
rustc_variance,
rust_eh_personality,
nnethercote marked this conversation as resolved.
Show resolved Hide resolved
rust_eh_register_frames,
rust_eh_unregister_frames,
rustfmt,
rust_oom,
rvalue_static_promotion,
Expand Down
Loading