employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: sym::SymbolStr: Remove 

This removes `SymbolStr` in favor of, simply, `&'static str`.

The abstraction provided no additional safety since the slice was trivially
extracted (and commonly, in practice), and was inconvenient to work with.

This is part of a process of relaxing lookups so that symbols can be
conveniently displayed in errors; rather than trying to prevent the
developer from doing something bad, we'll just rely on conventions, hope
that it doesn't happen, and if it does, address it either at that time or
when it shows up in the profiler.
main
Mike Gerwitz 2021-10-11 12:58:48 -04:00
parent 68397f1413
commit 85909f1590
9 changed files with 40 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
tamer/src/ir/asg/base.rs
View File

@ -392,8 +392,8 @@ mod test {
UnresolvedError,
};
use crate::ir::legacyir::SymDtype;
use crate::sym::GlobalSymbolIntern;
use crate::sym::SymbolId;
use crate::sym::{GlobalSymbolIntern, SymbolStr};
use std::cell::RefCell;
#[derive(Debug, Default, PartialEq)]
@ -622,7 +622,7 @@ mod test {
let node = sut.declare(sym, IdentKind::Meta, src.clone())?;
let obj = sut.get(node).unwrap();
let terr = TransitionError::ExternResolution {
name: SymbolStr::test_from_str("test fail"),
name: &"test fail",
expected: IdentKind::Meta,
given: IdentKind::Meta,
};
@ -687,7 +687,7 @@ mod test {
// It doesn't matter that this isn't the error that'll actually be
// returned, as long as it's some sort of TransitionError.
let terr = TransitionError::ExternResolution {
name: SymbolStr::test_from_str("test fail"),
name: &"test fail",
expected: IdentKind::Meta,
given: IdentKind::Meta,
};

24
tamer/src/ir/asg/object.rs
View File

@ -24,7 +24,7 @@
use super::ident::IdentKind;
use crate::ir::legacyir::SymAttrs;
use crate::sym::{GlobalSymbolResolve, SymbolId, SymbolStr};
use crate::sym::{GlobalSymbolResolve, SymbolId};
use std::result::Result;
pub type TransitionResult<T> = Result<T, (T, TransitionError)>;
@ -471,25 +471,25 @@ impl IdentObjectState<IdentObject> for IdentObject {
pub enum TransitionError {
/// Attempted to redeclare a concrete, non-virtual identifier without an
/// override.
Redeclare { name: SymbolStr<'static> },
Redeclare { name: &'static str },
/// Extern resolution failure.
///
/// An extern could not be resolved because the provided identifier had
/// a type that is incompatible with the extern definition.
ExternResolution {
name: SymbolStr<'static>,
name: &'static str,
expected: IdentKind,
given: IdentKind,
},
/// Attempt to override a non-virtual identifier.
NonVirtualOverride { name: SymbolStr<'static> },
NonVirtualOverride { name: &'static str },
/// Overriding a virtual identifier failed due to an incompatible
/// [`IdentKind`].
VirtualOverrideKind {
name: SymbolStr<'static>,
name: &'static str,
existing: IdentKind,
given: IdentKind,
},
@ -553,17 +553,17 @@ impl std::error::Error for TransitionError {
#[derive(Clone, Debug, PartialEq)]
pub enum UnresolvedError {
/// Expected identifier is missing and nothing about it is known.
Missing { name: SymbolStr<'static> },
Missing { name: &'static str },
/// Expected identifier has not yet been resolved with a concrete
/// definition.
Extern {
/// Identifier name.
name: SymbolStr<'static>,
name: &'static str,
/// Expected identifier type.
kind: IdentKind,
/// Name of package where the extern was defined.
pkg_name: Option<SymbolStr<'static>>,
pkg_name: Option<&'static str>,
},
}
@ -583,7 +583,7 @@ impl std::fmt::Display for UnresolvedError {
"unresolved extern `{}` of type `{}`, declared in `{}`",
name,
kind,
pkg_name.as_ref().map(|s| s.as_str()).unwrap_or("<unknown>"),
pkg_name.as_ref().unwrap_or(&"<unknown>"),
),
}
}
@ -988,7 +988,7 @@ mod test {
fn resolved_on_extern_error_fmt_without_pkg() {
let meta = IdentKind::Meta;
let err = UnresolvedError::Extern {
name: SymbolStr::test_from_str("foo"),
name: &"foo",
kind: IdentKind::Meta,
pkg_name: None,
};
@ -1003,10 +1003,10 @@ mod test {
#[test]
fn resolved_on_extern_error_fmt_with_pkg() {
let meta = IdentKind::Meta;
let pkg = SymbolStr::test_from_str("pkg");
let pkg = &"pkg";
let err = UnresolvedError::Extern {
name: SymbolStr::test_from_str("foo"),
name: &"foo",
kind: IdentKind::Meta,
pkg_name: Some(pkg.clone()),
};

2
tamer/src/ir/legacyir.rs
View File

@ -295,7 +295,7 @@ impl Into<SymbolId> for SymDtype {
impl AsRef<str> for SymDtype {
/// Produce `xmlo`-compatible representation.
fn as_ref(&self) -> &str {
self.as_sym().lookup_str().as_str()
self.as_sym().lookup_str()
}
}

2
tamer/src/ld/poc.rs
View File

@ -155,7 +155,7 @@ pub fn graphml(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
vec![
("label".into(), name.into()),
("kind".into(), kind.lookup_str().as_str().into()),
("kind".into(), kind.lookup_str().into()),
("generated".into(), generated.into()),
]
}));

6
tamer/src/obj/xmlo/asg_builder.rs
View File

@ -72,7 +72,7 @@ use crate::ir::asg::{
Asg, AsgError, IdentKind, IdentKindError, IdentObjectState, IndexType,
ObjectRef, Source,
};
use crate::sym::{GlobalSymbolResolve, SymbolId, SymbolStr};
use crate::sym::{GlobalSymbolResolve, SymbolId};
use std::collections::HashSet;
use std::convert::TryInto;
use std::error::Error;
@ -292,13 +292,13 @@ pub enum AsgBuilderError {
AsgError(AsgError),
/// Fragment encountered for an unknown identifier.
MissingFragmentIdent(SymbolStr<'static>),
MissingFragmentIdent(&'static str),
/// Eligibility classification references unknown identifier.
///
/// This is generated by the compiler and so should never happen.
/// (That's not to say that it won't, but it shouldn't.)
BadEligRef(SymbolStr<'static>),
BadEligRef(&'static str),
}
impl Display for AsgBuilderError {

8
tamer/src/obj/xmlo/reader/mod.rs
View File

@ -91,7 +91,7 @@
//! XmloEvent::SymDecl(sym, attrs) => syms.push((sym, attrs.ty)),
//! XmloEvent::SymDeps(sym, symdeps) => deps.push((sym, symdeps)),
//! XmloEvent::Fragment(sym, text) =>
//! fragments.push((sym, text.lookup_str().as_str())),
//! fragments.push((sym, text.lookup_str())),
//!
//! // Do not read past end of header.
//! XmloEvent::Eoh => break,
@ -137,9 +137,7 @@
//! ```
use crate::ir::legacyir::{PackageAttrs, SymAttrs, SymType};
use crate::sym::{
GlobalSymbolInternUnchecked, GlobalSymbolResolve, SymbolId, SymbolStr,
};
use crate::sym::{GlobalSymbolInternUnchecked, GlobalSymbolResolve, SymbolId};
#[cfg(test)]
use crate::test::quick_xml::MockBytesStart as BytesStart;
#[cfg(test)]
@ -801,7 +799,7 @@ pub enum XmloError {
/// A `preproc:fragment` element was found, but is missing `@id`.
UnassociatedFragment,
/// A `preproc:fragment` element was found, but is missing `text()`.
MissingFragmentText(SymbolStr<'static>),
MissingFragmentText(&'static str),
}
impl From<InnerXmlError> for XmloError {

18
tamer/src/sym/interner.rs
View File

@ -67,7 +67,6 @@
//! assert_eq!("foo", interner.index_lookup(ia).unwrap());
//! ```
use super::symbol::SymbolStr;
use super::{SymbolId, SymbolIndexSize};
use crate::global;
use bumpalo::Bump;
@ -85,7 +84,7 @@ use std::hash::BuildHasher;
/// allowing symbols to be compared for equality cheaply by comparing
/// integers.
/// Symbol locations in memory are fixed for the lifetime of the interner,
/// and can be retrieved as [`SymbolStr`] using
/// and can be retrieved as [`str`] using
/// [`index_lookup`](Interner::index_lookup).
///
/// If you care whether a value has been interned yet or not,
@ -141,11 +140,15 @@ pub trait Interner<'i, Ix: SymbolIndexSize> {
/// Look up a symbol's string value by its [`SymbolId`].
///
/// This will always return a [`SymbolStr`] as long as the provided
/// This will always return a [`str`] as long as the provided
/// `index` represents a symbol interned with this interner.
/// If the index is not found,
/// the result is [`None`].
fn index_lookup(&'i self, index: SymbolId<Ix>) -> Option<SymbolStr<'i>>;
///
/// [`str`] requires significantly more storage than an appropriate
/// [`SymbolId`] and should only be used when a string value must be
/// written (e.g. to a file or displayed to the user).
fn index_lookup(&'i self, index: SymbolId<Ix>) -> Option<&'i str>;
/// Intern an assumed-UTF-8 slice of bytes or return an existing
/// [`SymbolId`].
@ -339,11 +342,8 @@ where
self.map.borrow().len()
}
fn index_lookup(&'i self, index: SymbolId<Ix>) -> Option<SymbolStr<'i>> {
self.strings
.borrow()
.get(index.as_usize())
.map(|str| SymbolStr::from_interned_slice(*str))
fn index_lookup(&'i self, index: SymbolId<Ix>) -> Option<&'i str> {
self.strings.borrow().get(index.as_usize()).map(|str| *str)
}
}

12
tamer/src/sym/mod.rs
View File

@ -62,7 +62,7 @@
//! assert_ne!(foo, "bar".intern());
//!
//! // Interned slices can be looked up by their symbol id.
//! assert_eq!("foo", foo.lookup_str().as_str());
//! assert_eq!("foo", foo.lookup_str());
//! ```
//!
//! What Is String Interning?
@ -131,8 +131,6 @@
//! The string associated with a [`SymbolId`] can be looked up from the pool
//! using [`GlobalSymbolResolve::lookup_str`] for global interners,
//! or [`Interner::index_lookup`] otherwise.
//! Interned strings are represented by [`SymbolStr`],
//! which can be dereferenced into [`&str`].
//! Symbols allocated using a global interner will have a `'static`
//! lifetime.
//!
@ -177,7 +175,7 @@
//! a part of Rust itself,
//! and treated no differently than other core memory allocation.
//!
//! All [`SymbolStr`] objects returned from global interners hold a
//! All [`str`] objects returned from global interners hold a
//! `'static` lifetime to simplify lifetime management and borrowing.
//! However,
//! these should not be used in place of [`SymbolId`] if the string value
@ -190,7 +188,7 @@
//! - [`GlobalSymbolResolve`] provides a `lookup_str` method on
//! [`SymbolId`] which resolves the symbol using the appropriate
//! global interner,
//! producing a [`SymbolStr`] holding a reference to the `'static`
//! producing a [`str`] holding a reference to the `'static`
//! string slice within the pool.
//!
//! These traits are intentionally separate so that it is clear how a
@ -200,7 +198,7 @@
//!
//! TAMER does not currently utilize threads,
//! and global interners are never dropped,
//! and so [`SymbolStr`] will always refer to a valid string.
//! and so [`str`] will always refer to a valid string.
//!
//! There is no mechanism preventing [`SymbolId`] from one interner from
//! being used with another beyond [`SymbolIndexSize`] bounds;
@ -357,5 +355,5 @@ pub use interner::{
};
pub use symbol::{
GlobalSymbolIntern, GlobalSymbolInternUnchecked, GlobalSymbolResolve,
SymbolId, SymbolIndexSize, SymbolStr,
SymbolId, SymbolIndexSize,
};

83
tamer/src/sym/symbol.rs
View File

@ -24,7 +24,7 @@
use super::{DefaultInterner, Interner};
use crate::global;
use std::convert::{TryFrom, TryInto};
use std::fmt::{self, Debug, Display};
use std::fmt::Debug;
use std::hash::Hash;
use std::num::{NonZeroU16, NonZeroU32};
use std::ops::Deref;
@ -44,9 +44,6 @@ use std::thread::LocalKey;
///
/// Symbol Strings
/// ==============
/// [`SymbolId`] intentionally omits the [`Display`] trait to ensure that
/// compile-time errors occur when symbols are used in contexts where
/// strings are expected.
/// To resolve a [`SymbolId`] into the string that it represents,
/// see either [`GlobalSymbolResolve::lookup_str`] or
/// [`Interner::index_lookup`].
@ -179,80 +176,6 @@ thread_local! {
supported_symbol_index!(u16, NonZeroU16, Static16Interner, INTERNER_16);
supported_symbol_index!(u32, NonZeroU32, Static32Interner, INTERNER_32);
/// A string retrieved from the intern pool using a [`SymbolId`].
///
/// The lifetime of the inner string is constrained to the lifetime of the
/// interner itself.
/// For global interners,
/// this means that the string slice has a `'static` lifetime.
///
/// [`SymbolStr`] requires significantly more storage than an appropriate
/// [`SymbolId`] and should only be used when a string value must be
/// written (e.g. to a file or displayed to the user).
///
/// This value is intended to be short-lived.
#[derive(Debug, Default, Clone)]
pub struct SymbolStr<'i>(&'i str);
impl<'i> SymbolStr<'i> {
pub fn as_str(&self) -> &'i str {
self.0
}
/// Create a [`SymbolStr`] from a string for testing.
///
/// _This function is only available for tests for convenience!_
/// `SymbolStr` must always represent a real, interned string in
/// non-test code.
#[cfg(test)]
pub fn test_from_str(s: &'i str) -> Self {
SymbolStr(s)
}
}
impl<'i> SymbolStr<'i> {
pub(super) fn from_interned_slice(slice: &'i str) -> SymbolStr<'i> {
SymbolStr(slice)
}
}
impl<'i, T: Deref<Target = str>> PartialEq<T> for SymbolStr<'i> {
fn eq(&self, other: &T) -> bool {
self.0 == other.deref()
}
}
impl PartialEq<SymbolStr<'_>> for &str {
fn eq(&self, other: &SymbolStr<'_>) -> bool {
*self == other.0
}
}
impl<'i> Display for SymbolStr<'i> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}
// Once we have unsafe_impls stabalized,
// we should prevent `SymbolStr` from crossing threads.
// TAMER does not use threads at the time of writing,
// so this isn't a practical concern.
// If we _do_ want to pass between threads,
// we need to ensure the thread holding the interner lives longer than all
// other threads.
//impl<'i> !Send for SymbolStr<'i> {}
//impl<'i> !Sync for SymbolStr<'i> {}
impl<'i> Deref for SymbolStr<'i> {
type Target = str;
#[inline]
fn deref(&self) -> &'i str {
self.as_str()
}
}
/// Acquire a static reference to a global interner.
///
/// Global interners are static and thread-local.
@ -301,11 +224,11 @@ pub trait GlobalSymbolResolve {
/// lookup is performed on the global interner pool,
/// which requires locking and so comes at a (small) cost.
/// This shouldn't be done more than is necessary.
fn lookup_str(&self) -> SymbolStr<'static>;
fn lookup_str(&self) -> &'static str;
}
impl<Ix: SymbolIndexSize> GlobalSymbolResolve for SymbolId<Ix> {
fn lookup_str(&self) -> SymbolStr<'static> {
fn lookup_str(&self) -> &'static str {
Ix::with_static_interner(|interner| {
interner.index_lookup(*self).unwrap()
})