Skip to content

Commit

Permalink
Remove use of SmartString
Browse files Browse the repository at this point in the history 
The SmartString library uses pointer obfuscation tricks internally which
hide GC pointers from alloy. We must therefore remove it as it is
incompatible with conservative pointer scanning.
  • Loading branch information
@jacob-hughes
jacob-hughes committed Nov 21, 2023
1 parent 676db21 commit 0e4445f
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 71 deletions.
52 changes: 23 additions & 29 deletions src/lib/compiler/ast_to_instrs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use itertools::Itertools;
use lrlex::{DefaultLexerTypes, LRNonStreamingLexer};
use lrpar::{NonStreamingLexer, Span};
use num_bigint::BigInt;
use smartstring::alias::String as SmartString;

use std::gc::{Gc, NonFinalizable};

use crate::{
Expand All @@ -31,10 +31,10 @@ const CLOSURE_RETURN_VAR: &str = "$$closurereturn$$";
pub struct Compiler<'a, 'input> {
lexer: &'a LRNonStreamingLexer<'a, 'input, DefaultLexerTypes<StorageT>>,
path: &'a Path,
upvars_stack: Vec<Vec<(SmartString, UpVarDef)>>,
upvars_stack: Vec<Vec<(String, UpVarDef)>>,
/// The stack of local variables at the current point of evaluation. Maps variable names to
/// `(captured, var idx)`.
locals_stack: Vec<HashMap<SmartString, (bool, usize)>>,
locals_stack: Vec<HashMap<String, (bool, usize)>>,
blocks_stack: Vec<Vec<Gc<Function>>>,
/// Since SOM's "^" operator returns from the enclosed method, we need to track whether we are
/// in a closure -- and, if so, how many nested closures we are inside at the current point of
Expand All @@ -58,7 +58,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
lexer: &LRNonStreamingLexer<DefaultLexerTypes<StorageT>>,
path: &Path,
astcls: &ast::Class,
) -> Result<(SmartString, Val), String> {
) -> Result<(String, Val), String> {
let mut compiler = Compiler {
lexer,
path,
Expand All @@ -68,7 +68,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
closure_depth: 0,
};

let name = SmartString::from(lexer.span_str(astcls.name));
let name = String::from(lexer.span_str(astcls.name));
let (supercls_val, supercls_meta_val) = if name.as_str() != "Object" {
let supercls_val = if let Some(n) = astcls.supername.map(|x| lexer.span_str(x)) {
match vm.load_class(n) {
Expand Down Expand Up @@ -159,7 +159,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
&mut self,
vm: &mut VM,
lexer: &'a LRNonStreamingLexer<DefaultLexerTypes<StorageT>>,
name: SmartString,
name: String,
supercls_val: Val,
ast_inst_vars: &[Span],
ast_methods: &[ast::Method],
Expand Down Expand Up @@ -190,7 +190,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
};
for var in ast_inst_vars {
let vars_len = inst_vars.len();
let n = SmartString::from(lexer.span_str(*var));
let n = String::from(lexer.span_str(*var));
match inst_vars.entry(n) {
hash_map::Entry::Occupied(_) => {
return Err(vec![(
Expand Down Expand Up @@ -233,7 +233,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
let inst_vars_map = self.locals_stack[0]
.iter()
.map(|(k, v)| (k.clone(), v.1))
.collect::<HashMap<SmartString, usize>>();
.collect::<HashMap<String, usize>>();
self.locals_stack.pop();
if !errs.is_empty() {
return Err(errs);
Expand All @@ -257,27 +257,21 @@ impl<'a, 'input> Compiler<'a, 'input> {
Ok(cls_val)
}

fn c_method(
&mut self,
vm: &mut VM,
astmeth: &ast::Method,
) -> CompileResult<(SmartString, Val)> {
fn c_method(&mut self, vm: &mut VM, astmeth: &ast::Method) -> CompileResult<(String, Val)> {
let (name, args) = match astmeth.name {
ast::MethodName::BinaryOp(op, arg) => {
let arg_v = match arg {
Some(l) => vec![l],
None => vec![],
};
((op, SmartString::from(self.lexer.span_str(op))), arg_v)
}
ast::MethodName::Id(span) => {
((span, SmartString::from(self.lexer.span_str(span))), vec![])
((op, String::from(self.lexer.span_str(op))), arg_v)
}
ast::MethodName::Id(span) => ((span, String::from(self.lexer.span_str(span))), vec![]),
ast::MethodName::Keywords(ref pairs) => {
let name = pairs
.iter()
.map(|x| self.lexer.span_str(x.0))
.collect::<SmartString>();
.collect::<String>();
let args = pairs.iter().map(|x| x.1).collect::<Vec<_>>();
((pairs[0].0, name), args)
}
Expand Down Expand Up @@ -444,10 +438,10 @@ impl<'a, 'input> Compiler<'a, 'input> {
) -> CompileResult<(usize, Vec<Gc<Function>>, Vec<UpVarDef>)> {
self.upvars_stack.push(Vec::new());
let mut vars = HashMap::new();
vars.insert(SmartString::from("self"), (false, 0));
vars.insert(String::from("self"), (false, 0));
let mut process_var = |var_sp: Span| {
let vars_len = vars.len();
let var_str = SmartString::from(self.lexer.span_str(var_sp));
let var_str = String::from(self.lexer.span_str(var_sp));
match vars.entry(var_str.clone()) {
hash_map::Entry::Occupied(_) => Err(vec![(
var_sp,
Expand Down Expand Up @@ -542,7 +536,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
ast::Expr::BinaryMsg { span, lhs, op, rhs } => {
let mut stack_size = self.c_expr(vm, lhs)?;
stack_size = max(stack_size, 1 + self.c_expr(vm, rhs)?);
let send_off = vm.add_send((SmartString::from(self.lexer.span_str(*op)), 1));
let send_off = vm.add_send((String::from(self.lexer.span_str(*op)), 1));
let instr = match lhs {
box ast::Expr::UnaryMsg {
receiver: box ast::Expr::VarLookup(span2),
Expand Down Expand Up @@ -640,7 +634,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
msglist,
} => {
let mut max_stack = self.c_expr(vm, receiver)?;
let mut mn = SmartString::new();
let mut mn = String::new();
for (i, (kw, expr)) in msglist.iter().enumerate() {
mn.push_str(self.lexer.span_str(*kw));
let expr_stack = self.c_expr(vm, expr)?;
Expand Down Expand Up @@ -668,7 +662,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
} => {
let max_stack = self.c_expr(vm, receiver)?;
for (i, id) in ids.iter().enumerate() {
let send_off = vm.add_send((SmartString::from(self.lexer.span_str(*id)), 0));
let send_off = vm.add_send((String::from(self.lexer.span_str(*id)), 0));
let instr = match receiver {
box ast::Expr::VarLookup(span)
if i == 0 && self.lexer.span_str(*span) == "super" =>
Expand Down Expand Up @@ -697,7 +691,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
// "self" as an upvalue.
let locals_stack_len = self.locals_stack[1].len();
self.locals_stack[1]
.entry(SmartString::from(CLOSURE_RETURN_VAR))
.entry(String::from(CLOSURE_RETURN_VAR))
.or_insert((false, locals_stack_len));
match self.find_var_name(CLOSURE_RETURN_VAR) {
VarLookup::UpVar(upidx) => {
Expand All @@ -711,7 +705,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
}
ast::Expr::String(span) => {
let s_orig = self.lexer.span_str(*span);
let mut new_s = SmartString::new();
let mut new_s = String::new();
// Start by ignoring the beginning quote.
let mut i = '\"'.len_utf8();
// End by ignoring the beginning quote.
Expand Down Expand Up @@ -752,13 +746,13 @@ impl<'a, 'input> Compiler<'a, 'input> {
// XXX are there string escaping rules we need to take account of?
let s_orig = self.lexer.span_str(*span);
// Strip off the beginning/end quotes.
let s = SmartString::from(&s_orig[1..s_orig.len() - 1]);
let s = String::from(&s_orig[1..s_orig.len() - 1]);
let instr = Instr::Symbol(vm.add_symbol(s));
vm.instrs_push(instr, *span);
Ok(1)
}
ast::Expr::Symbol(span) => {
let s = SmartString::from(self.lexer.span_str(*span));
let s = String::from(self.lexer.span_str(*span));
let instr = Instr::Symbol(vm.add_symbol(s));
vm.instrs_push(instr, *span);
Ok(1)
Expand Down Expand Up @@ -814,7 +808,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
e.1
};
self.upvars_stack[depth + 1].push((
SmartString::from(name),
String::from(name),
UpVarDef {
capture_local: true,
upidx,
Expand All @@ -828,7 +822,7 @@ impl<'a, 'input> Compiler<'a, 'input> {
let mut upidx = i;
for j in depth + 1..self.upvars_stack.len() {
self.upvars_stack[j].push((
SmartString::from(name),
String::from(name),
UpVarDef {
capture_local: false,
upidx,
Expand Down
28 changes: 14 additions & 14 deletions src/lib/vm/core.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::{

use lrpar::Span;
use num_bigint::BigInt;
use smartstring::alias::String as SmartString;

use static_assertions::const_assert;
use std::gc::Gc;

Expand Down Expand Up @@ -78,24 +78,24 @@ pub struct VM {
/// The current known set of globals including those not yet assigned to: in other words, it is
/// expected that some entries of this `Vec` are illegal (i.e. created by `Val::illegal`).
globals: Vec<Val>,
reverse_globals: HashMap<SmartString, usize>,
reverse_globals: HashMap<String, usize>,
inline_caches: Vec<Option<(Val, Gc<Method>)>>,
/// `instrs` and `instr_span`s are always the same length: they are separated only because we
/// rarely access `instr_spans`.
instrs: Vec<Instr>,
instr_spans: Vec<Span>,
sends: Vec<(Gc<SmartString>, usize)>,
sends: Vec<(Gc<String>, usize)>,
/// reverse_sends is an optimisation allowing us to reuse sends: it maps a send `(String,
/// usize)` to a `usize` where the latter represents the index of the send in `sends`.
reverse_sends: HashMap<(Gc<SmartString>, usize), usize>,
reverse_sends: HashMap<(Gc<String>, usize), usize>,
stack: Gc<SOMStack>,
arbints: Vec<Val>,
strings: Vec<Val>,
/// reverse_strings is an optimisation allowing us to reuse strings: it maps a `String to a
/// `usize` where the latter represents the index of the string in `strings`.
reverse_strings: HashMap<SmartString, usize>,
reverse_strings: HashMap<String, usize>,
symbols: Vec<Val>,
reverse_symbols: HashMap<SmartString, usize>,
reverse_symbols: HashMap<String, usize>,
time_at_start: Instant,
open_upvars: Option<Gc<UpVar>>,
}
Expand Down Expand Up @@ -850,7 +850,7 @@ impl VM {
Err(_) => {
return SendReturn::Err(VMError::new(
self,
VMErrorKind::InvalidInteger(SmartString::from(s)),
VMErrorKind::InvalidInteger(String::from(s)),
))
}
},
Expand All @@ -861,7 +861,7 @@ impl VM {
Err(_) => {
return SendReturn::Err(VMError::new(
self,
VMErrorKind::InvalidDouble(SmartString::from(s)),
VMErrorKind::InvalidDouble(String::from(s)),
))
}
}
Expand Down Expand Up @@ -1061,7 +1061,7 @@ impl VM {
let path = PathBuf::from(stry!(path_val.downcast::<String_>(self)).as_str());
match read_to_string(path) {
Ok(s) => {
let v = String_::new_str(self, SmartString::from(s));
let v = String_::new_str(self, String::from(s));
self.stack.push(v);
}
Err(_) => self.stack.push(self.nil),
Expand Down Expand Up @@ -1408,7 +1408,7 @@ impl VM {

/// Add the send `send` to the VM, returning its index. Note that sends are reused, so indexes
/// are also reused.
pub fn add_send(&mut self, send: (SmartString, usize)) -> usize {
pub fn add_send(&mut self, send: (String, usize)) -> usize {
// We want to avoid `clone`ing `send` in the (hopefully common) case of a cache hit, hence
// this slightly laborious dance and double-lookup.
let send = (Gc::new(send.0), send.1);
Expand All @@ -1432,7 +1432,7 @@ impl VM {

/// Add the string `s` to the VM, returning its index. Note that strings are reused, so indexes
/// are also reused.
pub fn add_string(&mut self, s: SmartString) -> usize {
pub fn add_string(&mut self, s: String) -> usize {
// We want to avoid `clone`ing `s` in the (hopefully common) case of a cache hit, hence
// this slightly laborious dance and double-lookup.
if let Some(i) = self.reverse_strings.get(&s) {
Expand All @@ -1448,7 +1448,7 @@ impl VM {

/// Add the symbol `s` to the VM, returning its index. Note that symbols are reused, so indexes
/// are also reused.
pub fn add_symbol(&mut self, s: SmartString) -> usize {
pub fn add_symbol(&mut self, s: String) -> usize {
// We want to avoid `clone`ing `s` in the (hopefully common) case of a cache hit, hence
// this slightly laborious dance and double-lookup.
if let Some(i) = self.reverse_symbols.get(&s) {
Expand All @@ -1469,7 +1469,7 @@ impl VM {
*i
} else {
let len = self.globals.len();
self.reverse_globals.insert(SmartString::from(s), len);
self.reverse_globals.insert(String::from(s), len);
self.globals.push(Val::illegal());
len
}
Expand Down Expand Up @@ -1516,7 +1516,7 @@ impl VM {
self.globals[*i] = v;
} else {
self.reverse_globals
.insert(SmartString::from(name), self.globals.len());
.insert(String::from(name), self.globals.len());
self.globals.push(v);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/lib/vm/error.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{fs::read_to_string, io::stderr};

use lrpar::Span;
use smartstring::alias::String as SmartString;

use std::gc::Gc;
use termion::{is_tty, style};

Expand Down Expand Up @@ -163,9 +163,9 @@ pub enum VMErrorKind {
got_cls: Val,
},
/// Tried to convert an invalid string to a Double.
InvalidDouble(SmartString),
InvalidDouble(String),
/// Tried to convert an invalid string to an Integer.
InvalidInteger(SmartString),
InvalidInteger(String),
/// Tried to access a global before it being initialised.
InvalidSymbol,
/// Tried to do a shl or shr with a value below zero.
Expand All @@ -182,7 +182,7 @@ pub enum VMErrorKind {
/// Tried to do a shl that would overflow memory and/or not fit in the required integer size.
ShiftTooBig,
/// An unknown global.
UnknownGlobal(SmartString),
UnknownGlobal(String),
/// An unknown method.
UnknownMethod,
/// Tried calling a method with the wrong number of arguments.
Expand Down
9 changes: 4 additions & 5 deletions src/lib/vm/objects/class.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use std::{
str,
};

use smartstring::alias::String as SmartString;
use std::gc::{Gc, NonFinalizable};

use crate::vm::{
Expand All @@ -26,14 +25,14 @@ pub struct Class {
/// Offset to this class's instructions in VM::instrs.
pub instrs_off: usize,
supercls: Cell<Val>,
pub inst_vars_map: NonFinalizable<HashMap<SmartString, usize>>,
pub inst_vars_map: NonFinalizable<HashMap<String, usize>>,
/// A SOM Array of methods (though note that it is *not* guaranteed that these definitely point
/// to SOM `Method` instances -- anything can be stored in this array!).
methods: Val,
/// A map from method names to indexes into the methods SOM Array. Note that indexes are stored
/// with SOM indexing (starting from 1). We guarantee that the indexes are valid indexes for
/// the `methods` array.
methods_map: NonFinalizable<HashMap<SmartString, usize>>,
methods_map: NonFinalizable<HashMap<String, usize>>,
inst_vars: UnsafeCell<Vec<Val>>,
}

Expand Down Expand Up @@ -94,9 +93,9 @@ impl Class {
path: PathBuf,
instrs_off: usize,
supercls: Val,
inst_vars_map: NonFinalizable<HashMap<SmartString, usize>>,
inst_vars_map: NonFinalizable<HashMap<String, usize>>,
methods: Val,
methods_map: NonFinalizable<HashMap<SmartString, usize>>,
methods_map: NonFinalizable<HashMap<String, usize>>,
) -> Val {
#[cfg(debug_assertions)]
{
Expand Down
7 changes: 2 additions & 5 deletions src/lib/vm/objects/double.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{collections::hash_map::DefaultHasher, hash::Hasher};

use num_bigint::BigInt;
use num_traits::{FromPrimitive, ToPrimitive, Zero};
use smartstring::alias::String as SmartString;

use std::gc::Gc;

use crate::vm::{
Expand Down Expand Up @@ -33,10 +33,7 @@ impl Obj for Double {

fn to_strval(self: Gc<Self>, vm: &mut VM) -> Result<Val, Box<VMError>> {
let mut buf = ryu::Buffer::new();
Ok(String_::new_str(
vm,
SmartString::from(buf.format(self.val)),
))
Ok(String_::new_str(vm, String::from(buf.format(self.val))))
}

fn hashcode(self: Gc<Self>) -> u64 {
Expand Down
Loading

0 comments on commit 0e4445f

Please sign in to comment.