From 738bb63cbf3d6cabfddc9c4651380ea21e85d7fa Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Sun, 11 Jun 2017 15:02:00 +0200 Subject: [PATCH] Allow Variants to be constructed with an explicit root The `Array` type do not store `Value` directly but instead creates `Value` types on demand. The actual data is still lives as long as the the array itself though so the `Variants` need to reflect that to work in deserialization --- vm/src/api/mod.rs | 87 ++++++++++++-------- vm/src/api/serde.rs | 186 +++++++++++++++++++++++++------------------ vm/src/core/mod.rs | 3 +- vm/src/lib.rs | 8 +- vm/src/primitives.rs | 4 +- vm/src/stack.rs | 10 ++- vm/src/thread.rs | 31 ++++---- vm/src/value.rs | 31 +++++++- 8 files changed, 227 insertions(+), 133 deletions(-) diff --git a/vm/src/api/mod.rs b/vm/src/api/mod.rs index d41e578c37..942b540b8d 100644 --- a/vm/src/api/mod.rs +++ b/vm/src/api/mod.rs @@ -40,6 +40,7 @@ pub enum ValueRef<'a> { Float(f64), String(&'a str), Data(Data<'a>), + Array(ArrayRef<'a>), Tag(VmTag), Userdata(&'a vm::Userdata), Internal, @@ -102,6 +103,10 @@ impl<'a> Data<'a> { pub fn get(&self, index: usize) -> Option> { self.0.fields.get(index).map(ValueRef::new) } + + pub fn get_variants(&self, index: usize) -> Option> { + unsafe { self.0.fields.get(index).map(|v| Variants::new(v)) } + } } /// Marker type representing a hole @@ -198,7 +203,7 @@ impl<'vm, T: VmType> Pushable<'vm> for Generic { } impl<'vm, T> Getable<'vm> for Generic { fn from_value(_: &'vm Thread, value: Variants) -> Option> { - Some(Generic::from(*value.0)) + Some(Generic::from(value.0)) } } @@ -321,7 +326,7 @@ impl<'vm, T: vm::Userdata> Pushable<'vm> for T { impl<'vm> Getable<'vm> for Value { fn from_value(_vm: &'vm Thread, value: Variants) -> Option { - Some(*value.0) + Some(value.0) } } @@ -675,8 +680,8 @@ where } impl<'s, 'vm, T: Copy + ArrayRepr> Getable<'vm> for &'s [T] { unsafe fn from_value_unsafe(_: &'vm Thread, value: Variants) -> Option { - match *value.0 { - Value::Array(ptr) => ptr.as_slice().map(|s| &*(s as *const _)), + match value.as_ref() { + ValueRef::Array(ptr) => ptr.0.as_slice().map(|s| &*(s as *const _)), _ => None, } } @@ -781,15 +786,15 @@ impl<'vm, T: Pushable<'vm>> Pushable<'vm> for Option { } impl<'vm, T: Getable<'vm>> Getable<'vm> for Option { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - match *value.0 { - Value::Data(data) => { - if data.tag == 0 { + match value.as_ref() { + ValueRef::Data(data) => { + if data.tag() == 0 { Some(None) } else { - T::from_value(vm, Variants(&data.fields[0])).map(Some) + T::from_value(vm, data.get_variants(0).unwrap()).map(Some) } } - Value::Tag(0) => Some(None), + ValueRef::Tag(0) => Some(None), _ => None, } } @@ -837,11 +842,11 @@ impl<'vm, T: Pushable<'vm>, E: Pushable<'vm>> Pushable<'vm> for StdResult impl<'vm, T: Getable<'vm>, E: Getable<'vm>> Getable<'vm> for StdResult { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - match *value.0 { - Value::Data(data) => { - match data.tag { - 0 => E::from_value(vm, Variants(&data.fields[0])).map(Err), - 1 => T::from_value(vm, Variants(&data.fields[0])).map(Ok), + match value.as_ref() { + ValueRef::Data(data) => { + match data.tag() { + 0 => E::from_value(vm, data.get_variants(0).unwrap()).map(Err), + 1 => T::from_value(vm, data.get_variants(0).unwrap()).map(Ok), _ => None, } } @@ -1016,13 +1021,30 @@ where impl<'vm, V> Getable<'vm> for OpaqueValue<&'vm Thread, V> { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - Some(OpaqueValue(vm.root_value(*value.0), PhantomData)) + Some(OpaqueValue(vm.root_value(value.0), PhantomData)) } } impl<'vm, V> Getable<'vm> for OpaqueValue { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - Some(OpaqueValue(vm.root_value(*value.0), PhantomData)) + Some(OpaqueValue(vm.root_value(value.0), PhantomData)) + } +} + +#[derive(Copy, Clone, Debug)] +pub struct ArrayRef<'vm>(&'vm ValueArray); + +impl<'vm> ArrayRef<'vm> { + pub fn get(&self, index: usize) -> Option { + if index < self.0.len() { + unsafe { Some(Variants::with_root(self.0.get(index), self)) } + } else { + None + } + } + + pub fn iter(&self) -> ::value::VariantIter { + self.0.variant_iter() } } @@ -1047,12 +1069,14 @@ impl<'vm, T> Array<'vm, T> { impl<'vm, T: for<'vm2> Getable<'vm2>> Array<'vm, T> { pub fn get(&self, index: VmInt) -> Option { - match *self.0 { - Value::Array(data) => { - let v = data.get(index as usize); - T::from_value(self.0.vm(), Variants(&v)) + unsafe { + match Variants::new(&self.0).as_ref() { + ValueRef::Array(data) => { + let v = data.get(index as usize).unwrap(); + T::from_value(self.0.vm(), v) + } + _ => None, } - _ => None, } } } @@ -1080,7 +1104,7 @@ where impl<'vm, T> Getable<'vm> for Array<'vm, T> { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - Some(Array(vm.root_value(*value.0), PhantomData)) + Some(Array(vm.root_value(value.0), PhantomData)) } } @@ -1089,7 +1113,7 @@ impl<'vm, T: Any> VmType for Root<'vm, T> { } impl<'vm, T: vm::Userdata> Getable<'vm> for Root<'vm, T> { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - match *value.0 { + match value.0 { Value::Userdata(data) => vm.root::(data).map(From::from), _ => None, } @@ -1101,7 +1125,7 @@ impl<'vm> VmType for RootStr<'vm> { } impl<'vm> Getable<'vm> for RootStr<'vm> { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { - match *value.0 { + match value.0 { Value::String(v) => Some(vm.root_string(v)), _ => None, } @@ -1160,9 +1184,10 @@ macro_rules! define_tuple { fn from_value(vm: &'vm Thread, value: Variants) -> Option<($($id),+)> { match value.as_ref() { ValueRef::Data(v) => { + assert!(v.len() == count!($($id),+)); let mut i = 0; let x = ( $( - { let a = $id::from_value(vm, Variants(&v.0.fields[i])); i += 1; a } + { let a = $id::from_value(vm, v.get_variants(i).unwrap()); i += 1; a } ),+ ); match x { ($(Some($id)),+) => Some(( $($id),+ )), @@ -1367,7 +1392,7 @@ pub mod record { T: GetableFieldList<'vm>, { fn from_value(vm: &'vm Thread, value: Variants) -> Option { - match *value.0 { + match value.0 { Value::Data(ref data) => { HList::<(F, A), T>::from_value(vm, &data.fields).map( |fields| { @@ -1539,7 +1564,7 @@ where impl<'vm, F> Getable<'vm> for Function<&'vm Thread, F> { fn from_value(vm: &'vm Thread, value: Variants) -> Option> { Some(Function { - value: vm.root_value(*value.0), + value: vm.root_value(value.0), _marker: PhantomData, }) //TODO not type safe } @@ -1548,7 +1573,7 @@ impl<'vm, F> Getable<'vm> for Function<&'vm Thread, F> { impl<'vm, F> Getable<'vm> for Function { fn from_value(vm: &'vm Thread, value: Variants) -> Option { Some(Function { - value: vm.root_value(*value.0), + value: vm.root_value(value.0), _marker: PhantomData, }) //TODO not type safe } @@ -1630,7 +1655,7 @@ where $($args: Getable<'vm> + 'vm,)* let (lock, ($($args,)*)) = { let stack = StackFrame::current(&mut context.stack); $(let $args = { - let x = $args::from_value_unsafe(vm, Variants(&stack[i])) + let x = $args::from_value_unsafe(vm, Variants::new(&stack[i])) .expect(stringify!(Argument $args)); i += 1; x @@ -1678,7 +1703,7 @@ where $($args: Getable<'vm> + 'vm,)* let (lock, ($($args,)*)) = { let stack = StackFrame::current(&mut context.stack); $(let $args = { - let x = $args::from_value_unsafe(vm, Variants(&stack[i])) + let x = $args::from_value_unsafe(vm, Variants::new(&stack[i])) .expect(stringify!(Argument $args)); i += 1; x @@ -1734,7 +1759,7 @@ impl<'vm, T, $($args,)* R> Function R> } fn return_value(vm: &Thread, value: Value) -> Result { - R::from_value(vm, Variants(&value)) + R::from_value(vm, Variants::new(&value)) .ok_or_else(|| { error!("Wrong type {:?}", value); Error::Message("Wrong type".to_string()) diff --git a/vm/src/api/serde.rs b/vm/src/api/serde.rs index b4d556d72e..f431b135e4 100644 --- a/vm/src/api/serde.rs +++ b/vm/src/api/serde.rs @@ -11,7 +11,8 @@ use value::{Def, Value}; use self::serde::ser::{self, Serialize}; pub fn to_value(thread: &Thread, value: &T) -> Result - where T: ?Sized + Serialize +where + T: ?Sized + Serialize, { let mut context = thread.context(); Ser(value).push(thread, &mut context)?; @@ -21,7 +22,8 @@ pub fn to_value(thread: &Thread, value: &T) -> Result pub struct Ser(pub T); impl<'vm, T> Pushable<'vm> for Ser - where T: Serialize +where + T: Serialize, { fn push(self, thread: &'vm Thread, context: &mut Context) -> Result<()> { let mut serializer = Serializer { @@ -34,7 +36,8 @@ impl<'vm, T> Pushable<'vm> for Ser impl ser::Error for Error { fn custom(msg: T) -> Self - where T: fmt::Display + where + T: fmt::Display, { Error::Message(format!("{}", msg)) } @@ -47,18 +50,17 @@ struct Serializer<'t> { impl<'t> Serializer<'t> { fn to_value(&mut self, value: T) -> Result<()> - where T: Pushable<'t> + where + T: Pushable<'t>, { value.push(self.thread, self.context) } fn alloc(&mut self, tag: VmTag, values: VmIndex) -> Result<()> { - let value = self.context - .gc - .alloc(Def { - tag: tag, - elems: &self.context.stack[self.context.stack.len() - values..], - })?; + let value = self.context.gc.alloc(Def { + tag: tag, + elems: &self.context.stack[self.context.stack.len() - values..], + })?; for _ in 0..values { self.context.stack.pop(); } @@ -190,7 +192,8 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { // what people expect when working with JSON. Other formats are encouraged // to behave more intelligently if possible. fn serialize_some(self, value: &T) -> Result - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(self) } @@ -204,11 +207,12 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { self.serialize_unit() } - fn serialize_unit_variant(self, - _name: &'static str, - variant_index: u32, - _variant: &'static str) - -> Result { + fn serialize_unit_variant( + self, + _name: &'static str, + variant_index: u32, + _variant: &'static str, + ) -> Result { self.context.stack.push(Value::Tag(variant_index)); Ok(()) } @@ -216,7 +220,8 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { // As is done here, serializers are encouraged to treat newtype structs as // insignificant wrappers around the data they contain. fn serialize_newtype_struct(self, _name: &'static str, value: &T) -> Result - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(self) } @@ -226,13 +231,15 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { // representation. // // Serialize this to JSON in externally tagged form as `{ NAME: VALUE }`. - fn serialize_newtype_variant(self, - _name: &'static str, - variant_index: u32, - _variant: &'static str, - value: &T) - -> Result - where T: ?Sized + Serialize + fn serialize_newtype_variant( + self, + _name: &'static str, + variant_index: u32, + _variant: &'static str, + value: &T, + ) -> Result + where + T: ?Sized + Serialize, { value.serialize(&mut *self)?; self.alloc(variant_index, 1) @@ -246,19 +253,21 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { self.serialize_seq(Some(len)) } - fn serialize_tuple_struct(self, - _name: &'static str, - len: usize) - -> Result { + fn serialize_tuple_struct( + self, + _name: &'static str, + len: usize, + ) -> Result { self.serialize_seq(Some(len)) } - fn serialize_tuple_variant(self, - _name: &'static str, - variant_index: u32, - _variant: &'static str, - _len: usize) - -> Result { + fn serialize_tuple_variant( + self, + _name: &'static str, + variant_index: u32, + _variant: &'static str, + _len: usize, + ) -> Result { Ok(RecordSerializer::new(self, variant_index)) } @@ -270,12 +279,13 @@ impl<'a, 'vm> ser::Serializer for &'a mut Serializer<'vm> { self.serialize_map(Some(len)) } - fn serialize_struct_variant(self, - _name: &'static str, - variant_index: u32, - _variant: &'static str, - _len: usize) - -> Result { + fn serialize_struct_variant( + self, + _name: &'static str, + variant_index: u32, + _variant: &'static str, + _len: usize, + ) -> Result { Ok(RecordSerializer::new(self, variant_index)) } } @@ -285,7 +295,8 @@ impl<'a, 'vm> ser::SerializeSeq for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_element(&mut self, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -302,7 +313,8 @@ impl<'a, 'vm> ser::SerializeTuple for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_element(&mut self, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -319,7 +331,8 @@ impl<'a, 'vm> ser::SerializeTupleStruct for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_field(&mut self, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -336,7 +349,8 @@ impl<'a, 'vm> ser::SerializeTupleVariant for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_field(&mut self, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -353,13 +367,15 @@ impl<'a, 'vm> ser::SerializeMap for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_key(&mut self, _key: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { Ok(()) } fn serialize_value(&mut self, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -376,7 +392,8 @@ impl<'a, 'vm> ser::SerializeStruct for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_field(&mut self, _key: &'static str, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -395,7 +412,8 @@ impl<'a, 'vm> ser::SerializeStructVariant for RecordSerializer<'a, 'vm> { type Error = Error; fn serialize_field(&mut self, _key: &'static str, value: &T) -> Result<()> - where T: ?Sized + Serialize + where + T: ?Sized + Serialize, { value.serialize(&mut **self)?; self.values += 1; @@ -413,7 +431,8 @@ mod tests { use thread::{RootedThread, ThreadInternal}; fn make_value<'vm, T>(thread: &'vm Thread, value: T) -> Value - where T: Pushable<'vm> + where + T: Pushable<'vm>, { let mut context = thread.context(); value.push(thread, &mut context).unwrap(); @@ -435,18 +454,23 @@ mod tests { #[test] fn record() { let thread = RootedThread::new(); - let actual = to_value(&thread, - &Record { - test: 123, - string: "abc".to_string(), - }) - .unwrap(); - assert_eq!(actual, - make_value(&thread, - record! { + let actual = to_value( + &thread, + &Record { + test: 123, + string: "abc".to_string(), + }, + ).unwrap(); + assert_eq!( + actual, + make_value( + &thread, + record! { test => 123, string => "abc" - })); + }, + ) + ); } #[derive(Serialize)] @@ -461,25 +485,33 @@ mod tests { let actual = to_value(&thread, &Enum::A("abc".to_string())).unwrap(); let s = make_value(&thread, "abc"); - assert_eq!(actual, - Value::Data(thread - .context() - .gc - .alloc(Def { - tag: 0, - elems: &[s], - }) - .unwrap())); + assert_eq!( + actual, + Value::Data( + thread + .context() + .gc + .alloc(Def { + tag: 0, + elems: &[s], + }) + .unwrap(), + ) + ); let actual = to_value(&thread, &Enum::B(1232)).unwrap(); - assert_eq!(actual, - Value::Data(thread - .context() - .gc - .alloc(Def { - tag: 1, - elems: &[Value::Int(1232)], - }) - .unwrap())); + assert_eq!( + actual, + Value::Data( + thread + .context() + .gc + .alloc(Def { + tag: 1, + elems: &[Value::Int(1232)], + }) + .unwrap(), + ) + ); } } diff --git a/vm/src/core/mod.rs b/vm/src/core/mod.rs index 6df5a0be76..035505952a 100644 --- a/vm/src/core/mod.rs +++ b/vm/src/core/mod.rs @@ -377,7 +377,8 @@ impl<'a, 'e> Translator<'a, 'e> { } } ast::Expr::IfElse(ref pred, ref if_true, ref if_false) => { - let alts: SmallVec<[_; 2]> = collect![ + let alts: SmallVec<[_; 2]> = + collect![ Alternative { pattern: Pattern::Constructor(self.bool_constructor(true), vec![]), expr: self.translate_alloc(if_true), diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 022b553640..fb40a69ce2 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -64,13 +64,17 @@ impl<'a> Variants<'a> { /// Creates a new `Variants` value which assumes that `value` is alive for the lifetime of the /// value pub unsafe fn new(value: &Value) -> Variants { - Variants(value) + Variants::with_root(*value, value) + } + + pub unsafe fn with_root(value: Value, _root: &T) -> Variants { + Variants(value, PhantomData) } /// Returns an instance of `ValueRef` which allows users to safely retrieve the interals of a /// value pub fn as_ref(&self) -> ValueRef { - ValueRef::new(self.0) + ValueRef::new(&self.0) } } diff --git a/vm/src/primitives.rs b/vm/src/primitives.rs index 65a229b665..daba936e28 100644 --- a/vm/src/primitives.rs +++ b/vm/src/primitives.rs @@ -80,7 +80,7 @@ fn array_append<'vm>( } }; RuntimeResult::Return( - Getable::from_value(lhs.vm(), Variants(&Value::Array(value))).expect("Array"), + Getable::from_value(lhs.vm(), Variants::new(&Value::Array(value))).expect("Array"), ) } @@ -126,7 +126,7 @@ fn string_append(lhs: WithVM<&str>, rhs: &str) -> RuntimeResult { } }; RuntimeResult::Return( - Getable::from_value(vm, Variants(&Value::String(value))).expect("Array"), + Getable::from_value(vm, Variants::new(&Value::String(value))).expect("Array"), ) } diff --git a/vm/src/stack.rs b/vm/src/stack.rs index cd4094e2dc..7402c0900e 100644 --- a/vm/src/stack.rs +++ b/vm/src/stack.rs @@ -213,10 +213,12 @@ impl<'a: 'b, 'b> StackFrame<'b> { } pub fn get_variants(&self, index: VmIndex) -> Option { - if index < self.len() { - Some(Variants(&self[index])) - } else { - None + unsafe { + if index < self.len() { + Some(Variants::new(&self[index])) + } else { + None + } } } diff --git a/vm/src/thread.rs b/vm/src/thread.rs index 7a4215ebd4..53bad969ed 100644 --- a/vm/src/thread.rs +++ b/vm/src/thread.rs @@ -461,9 +461,11 @@ impl Thread { // Finally check that type of the returned value is correct let expected = T::make_type(self); if check_signature(&*env, &expected, &actual) { - T::from_value(self, Variants(&value)).ok_or_else(|| { - Error::UndefinedBinding(name.into()) - }) + unsafe { + T::from_value(self, Variants::new(&value)).ok_or_else(|| { + Error::UndefinedBinding(name.into()) + }) + } } else { Err(Error::WrongType(expected, actual.into_owned())) } @@ -1834,19 +1836,18 @@ where F: FnOnce(T, T) -> Value, T: Getable<'b> + fmt::Debug, { - let r = stack.pop(); - let l = stack.pop(); - match ( - T::from_value(vm, Variants(&l)), - T::from_value(vm, Variants(&r)), - ) { - (Some(l), Some(r)) => { - let result = f(l, r); - // pushing numbers should never return an error so unwrap - stack.stack.push(result); + let (l, r) = { + let r = stack.get_variants(stack.len() - 1).unwrap(); + let l = stack.get_variants(stack.len() - 2).unwrap(); + match (T::from_value(vm, l), T::from_value(vm, r)) { + (Some(l), Some(r)) => (l, r), + _ => panic!("{:?} `op` {:?}", l, r), } - _ => panic!("{:?} `op` {:?}", l, r), - } + }; + let result = f(l, r); + stack.pop(); + stack.pop(); + stack.stack.push(result); } fn debug_instruction( diff --git a/vm/src/value.rs b/vm/src/value.rs index 389875286d..d082e8a8ba 100644 --- a/vm/src/value.rs +++ b/vm/src/value.rs @@ -16,7 +16,7 @@ use compiler::DebugInfo; use gc::{Gc, GcPtr, Generation, Traverseable, DataDef, Move, WriteOnly}; use array::Array; use thread::{Thread, Status}; -use {Error, Result}; +use {Error, Result, Variants}; use self::Value::{Int, Float, String, Function, PartialApplication, Closure}; @@ -802,6 +802,28 @@ impl<'a> Iterator for Iter<'a> { } } +pub struct VariantIter<'a> { + array: &'a ValueArray, + index: usize, +} +impl<'a> Iterator for VariantIter<'a> { + type Item = Variants<'a>; + fn next(&mut self) -> Option { + if self.index < self.array.len() { + let value = self.array.get(self.index); + self.index += 1; + Some(unsafe { Variants::with_root(value, self.array) }) + } else { + None + } + } + + fn size_hint(&self) -> (usize, Option) { + let i = self.array.len() - self.index; + (i, Some(i)) + } +} + impl Traverseable for ValueArray { fn traverse(&self, gc: &mut Gc) { on_array!(*self, |array: &Array<_>| array.traverse(gc)) @@ -839,6 +861,13 @@ impl ValueArray { } } + pub fn variant_iter(&self) -> VariantIter { + VariantIter { + array: self, + index: 0, + } + } + pub fn size_of(repr: Repr, len: usize) -> usize { ::std::mem::size_of::() + repr.size_of() * len }