diff --git a/tamer/src/ir/asg/base.rs b/tamer/src/ir/asg/base.rs index 4cc99de4..10367712 100644 --- a/tamer/src/ir/asg/base.rs +++ b/tamer/src/ir/asg/base.rs @@ -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, }; diff --git a/tamer/src/ir/asg/object.rs b/tamer/src/ir/asg/object.rs index 4015b8a1..585d750b 100644 --- a/tamer/src/ir/asg/object.rs +++ b/tamer/src/ir/asg/object.rs @@ -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 = Result; @@ -471,25 +471,25 @@ impl IdentObjectState 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>, + 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(""), + pkg_name.as_ref().unwrap_or(&""), ), } } @@ -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()), }; diff --git a/tamer/src/ir/legacyir.rs b/tamer/src/ir/legacyir.rs index 93399bd1..dafa8036 100644 --- a/tamer/src/ir/legacyir.rs +++ b/tamer/src/ir/legacyir.rs @@ -295,7 +295,7 @@ impl Into for SymDtype { impl AsRef for SymDtype { /// Produce `xmlo`-compatible representation. fn as_ref(&self) -> &str { - self.as_sym().lookup_str().as_str() + self.as_sym().lookup_str() } } diff --git a/tamer/src/ld/poc.rs b/tamer/src/ld/poc.rs index 31c81e9b..d6b5e3a4 100644 --- a/tamer/src/ld/poc.rs +++ b/tamer/src/ld/poc.rs @@ -155,7 +155,7 @@ pub fn graphml(package_path: &str, output: &str) -> Result<(), Box> { vec![ ("label".into(), name.into()), - ("kind".into(), kind.lookup_str().as_str().into()), + ("kind".into(), kind.lookup_str().into()), ("generated".into(), generated.into()), ] })); diff --git a/tamer/src/obj/xmlo/asg_builder.rs b/tamer/src/obj/xmlo/asg_builder.rs index 0f77ba2c..0cec7657 100644 --- a/tamer/src/obj/xmlo/asg_builder.rs +++ b/tamer/src/obj/xmlo/asg_builder.rs @@ -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 { diff --git a/tamer/src/obj/xmlo/reader/mod.rs b/tamer/src/obj/xmlo/reader/mod.rs index 208421f5..4aaa8b59 100644 --- a/tamer/src/obj/xmlo/reader/mod.rs +++ b/tamer/src/obj/xmlo/reader/mod.rs @@ -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 for XmloError { diff --git a/tamer/src/sym/interner.rs b/tamer/src/sym/interner.rs index 9c9974dd..3b3d4e89 100644 --- a/tamer/src/sym/interner.rs +++ b/tamer/src/sym/interner.rs @@ -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) -> Option>; + /// + /// [`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) -> 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) -> Option> { - self.strings - .borrow() - .get(index.as_usize()) - .map(|str| SymbolStr::from_interned_slice(*str)) + fn index_lookup(&'i self, index: SymbolId) -> Option<&'i str> { + self.strings.borrow().get(index.as_usize()).map(|str| *str) } } diff --git a/tamer/src/sym/mod.rs b/tamer/src/sym/mod.rs index 1e4b481e..9cffb9cc 100644 --- a/tamer/src/sym/mod.rs +++ b/tamer/src/sym/mod.rs @@ -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, }; diff --git a/tamer/src/sym/symbol.rs b/tamer/src/sym/symbol.rs index b269f5ca..8feecd57 100644 --- a/tamer/src/sym/symbol.rs +++ b/tamer/src/sym/symbol.rs @@ -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> PartialEq for SymbolStr<'i> { - fn eq(&self, other: &T) -> bool { - self.0 == other.deref() - } -} - -impl PartialEq> 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 GlobalSymbolResolve for SymbolId { - fn lookup_str(&self) -> SymbolStr<'static> { + fn lookup_str(&self) -> &'static str { Ix::with_static_interner(|interner| { interner.index_lookup(*self).unwrap() })