From 4cd30455debcf631468bc1d7c9a3acd5a9505979 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 22 Jun 2020 20:56:18 -0400 Subject: [PATCH] Excise `ReturnValue<'gc>` from all `TObject` methods. Inspired by Dinnerbone's PR doing the exact same thing to AVM1. On AVM2 we have a bit of a subtle issue: the base implementation of `set_property_local` and `init_property_local` *must* return `ReturnValue`s to avoid double-borrows. Each implementation of `TObject` must resolve them before returning. --- core/src/avm2.rs | 72 ++++++++++--------------------- core/src/avm2/function.rs | 30 ++++++++----- core/src/avm2/globals/function.rs | 6 ++- core/src/avm2/object.rs | 42 +++++------------- core/src/avm2/scope.rs | 11 +++-- core/src/avm2/script_object.rs | 31 ++++++++----- 6 files changed, 82 insertions(+), 110 deletions(-) diff --git a/core/src/avm2.rs b/core/src/avm2.rs index 9f634ee65..85e875d70 100644 --- a/core/src/avm2.rs +++ b/core/src/avm2.rs @@ -641,10 +641,9 @@ impl<'gc> Avm2<'gc> { let receiver = self.pop().as_object().ok(); let function = self.pop().as_object()?; let base_proto = receiver.and_then(|r| r.proto()); + let value = function.call(receiver, &args, self, context, base_proto)?; - function - .call(receiver, &args, self, context, base_proto)? - .push(self); + self.push(value); Ok(()) } @@ -661,10 +660,9 @@ impl<'gc> Avm2<'gc> { .get_method(index.0) .ok_or_else(|| format!("Object method {} does not exist", index.0).into()); let base_proto = receiver.proto(); + let value = function?.call(Some(receiver), &args, self, context, base_proto)?; - function? - .call(Some(receiver), &args, self, context, base_proto)? - .push(self); + self.push(value); Ok(()) } @@ -685,12 +683,10 @@ impl<'gc> Avm2<'gc> { let base_proto = receiver.get_base_proto(&name)?; let function = receiver .get_property(receiver, &name, self, context)? - .resolve(self, context)? .as_object()?; + let value = function.call(Some(receiver), &args, self, context, base_proto)?; - function - .call(Some(receiver), &args, self, context, base_proto)? - .push(self); + self.push(value); Ok(()) } @@ -709,10 +705,10 @@ impl<'gc> Avm2<'gc> { .ok_or_else(|| format!("Could not find method {:?}", multiname.local_name()).into()); let function = receiver .get_property(receiver, &name?, self, context)? - .resolve(self, context)? .as_object()?; + let value = function.call(None, &args, self, context, None)?; - function.call(None, &args, self, context, None)?.push(self); + self.push(value); Ok(()) } @@ -733,12 +729,9 @@ impl<'gc> Avm2<'gc> { let base_proto = receiver.get_base_proto(&name)?; let function = receiver .get_property(receiver, &name, self, context)? - .resolve(self, context)? .as_object()?; - function - .call(Some(receiver), &args, self, context, base_proto)? - .resolve(self, context)?; + function.call(Some(receiver), &args, self, context, base_proto)?; Ok(()) } @@ -760,10 +753,9 @@ impl<'gc> Avm2<'gc> { self.system_prototypes.function, None, ); + let value = function.call(Some(receiver), &args, self, context, receiver.proto())?; - function - .call(Some(receiver), &args, self, context, receiver.proto())? - .push(self); + self.push(value); Ok(()) } @@ -796,12 +788,11 @@ impl<'gc> Avm2<'gc> { let function = base .get_property(receiver, &name?, self, context)? - .resolve(self, context)? .as_object()?; - function - .call(Some(receiver), &args, self, context, Some(base_proto))? - .push(self); + let value = function.call(Some(receiver), &args, self, context, Some(base_proto))?; + + self.push(value); Ok(()) } @@ -834,12 +825,9 @@ impl<'gc> Avm2<'gc> { let function = base .get_property(receiver, &name?, self, context)? - .resolve(self, context)? .as_object()?; - function - .call(Some(receiver), &args, self, context, Some(base_proto))? - .resolve(self, context)?; + function.call(Some(receiver), &args, self, context, Some(base_proto))?; Ok(()) } @@ -866,9 +854,7 @@ impl<'gc> Avm2<'gc> { format!("Could not resolve property {:?}", multiname.local_name()).into() }); - let value = object - .get_property(object, &name?, self, context)? - .resolve(self, context)?; + let value = object.get_property(object, &name?, self, context)?; self.push(value); Ok(()) @@ -960,9 +946,7 @@ impl<'gc> Avm2<'gc> { .into() }); - let value = base - .get_property(object, &name?, self, context)? - .resolve(self, context)?; + let value = base.get_property(object, &name?, self, context)?; self.push(value); @@ -1191,12 +1175,10 @@ impl<'gc> Avm2<'gc> { self, context, )? - .resolve(self, context)? .as_object()?; let object = proto.construct(self, context, &args)?; - ctor.call(Some(object), &args, self, context, object.proto())? - .resolve(self, context)?; + ctor.call(Some(object), &args, self, context, object.proto())?; self.push(object); @@ -1219,7 +1201,6 @@ impl<'gc> Avm2<'gc> { }); let mut ctor = source .get_property(source, &ctor_name?, self, context)? - .resolve(self, context)? .as_object()?; let proto = ctor .get_property( @@ -1228,12 +1209,10 @@ impl<'gc> Avm2<'gc> { self, context, )? - .resolve(self, context)? .as_object()?; let object = proto.construct(self, context, &args)?; - ctor.call(Some(object), &args, self, context, Some(proto))? - .resolve(self, context)?; + ctor.call(Some(object), &args, self, context, Some(proto))?; self.push(object); @@ -1263,12 +1242,9 @@ impl<'gc> Avm2<'gc> { let function = base_proto .get_property(receiver, &name, self, context)? - .resolve(self, context)? .as_object()?; - function - .call(Some(receiver), &args, self, context, Some(base_proto))? - .resolve(self, context)?; + function.call(Some(receiver), &args, self, context, Some(base_proto))?; Ok(()) } @@ -1345,9 +1321,7 @@ impl<'gc> Avm2<'gc> { let (new_class, class_init) = FunctionObject::from_abc_class(self, context, class_entry, base_class, scope)?; - class_init - .call(Some(new_class), &[], self, context, None)? - .resolve(self, context)?; + class_init.call(Some(new_class), &[], self, context, None)?; self.push(new_class); @@ -1500,9 +1474,7 @@ impl<'gc> Avm2<'gc> { let name = object.get_enumerant_name(cur_index as u32); let value = if let Some(name) = name { - object - .get_property(object, &name, self, context)? - .resolve(self, context)? + object.get_property(object, &name, self, context)? } else { Value::Undefined }; diff --git a/core/src/avm2/function.rs b/core/src/avm2/function.rs index ee763390d..03b830dcd 100644 --- a/core/src/avm2/function.rs +++ b/core/src/avm2/function.rs @@ -284,7 +284,6 @@ impl<'gc> FunctionObject<'gc> { avm, context, )? - .resolve(avm, context)? .as_object() .map_err(|_| { let super_name = QName::from_abc_multiname( @@ -443,7 +442,7 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> { name: &QName, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { + ) -> Result, Error> { self.0 .read() .base @@ -457,11 +456,16 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> { value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { - self.0 + ) -> Result<(), Error> { + let rv = self + .0 .write(context.gc_context) .base - .set_property_local(reciever, name, value, avm, context) + .set_property_local(reciever, name, value, avm, context)?; + + rv.resolve(avm, context)?; + + Ok(()) } fn init_property_local( @@ -471,11 +475,16 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> { value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { - self.0 + ) -> Result<(), Error> { + let rv = self + .0 .write(context.gc_context) .base - .init_property_local(reciever, name, value, avm, context) + .init_property_local(reciever, name, value, avm, context)?; + + rv.resolve(avm, context)?; + + Ok(()) } fn is_property_overwritable(self, gc_context: MutationContext<'gc, '_>, name: &QName) -> bool { @@ -603,9 +612,10 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> { avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, base_proto: Option>, - ) -> Result, Error> { + ) -> Result, Error> { if let Some(exec) = &self.0.read().exec { - exec.exec(reciever, arguments, avm, context, base_proto) + exec.exec(reciever, arguments, avm, context, base_proto)? + .resolve(avm, context) } else { Err("Not a callable function!".into()) } diff --git a/core/src/avm2/globals/function.rs b/core/src/avm2/globals/function.rs index 7a07d31af..4f3fba60b 100644 --- a/core/src/avm2/globals/function.rs +++ b/core/src/avm2/globals/function.rs @@ -32,9 +32,11 @@ fn call<'gc>( if let Some(func) = func { if args.len() > 1 { - func.call(this, &args[1..], avm, context, base_proto) + Ok(func + .call(this, &args[1..], avm, context, base_proto)? + .into()) } else { - func.call(this, &[], avm, context, base_proto) + Ok(func.call(this, &[], avm, context, base_proto)?.into()) } } else { Err("Not a callable function".into()) diff --git a/core/src/avm2/object.rs b/core/src/avm2/object.rs index 4f4ac9120..908964ff0 100644 --- a/core/src/avm2/object.rs +++ b/core/src/avm2/object.rs @@ -2,7 +2,6 @@ use crate::avm2::function::{Avm2ClassEntry, Avm2MethodEntry, Executable, FunctionObject}; use crate::avm2::names::{Multiname, Namespace, QName}; -use crate::avm2::return_value::ReturnValue; use crate::avm2::scope::Scope; use crate::avm2::script_object::ScriptObject; use crate::avm2::value::{abc_default_value, Value}; @@ -33,7 +32,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy name: &QName, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error>; + ) -> Result, Error>; /// Retrieve a property by it's QName. fn get_property( @@ -42,7 +41,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy name: &QName, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { + ) -> Result, Error> { if !self.has_instantiated_property(name) { for abc_trait in self.get_trait(name)? { self.install_trait(avm, context, &abc_trait, reciever)?; @@ -59,7 +58,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy return proto.get_property(reciever, name, avm, context); } - Ok(Value::Undefined.into()) + Ok(Value::Undefined) } /// Retrieve the base prototype that a particular QName trait is defined in. @@ -79,9 +78,6 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy } /// Set a property on this specific object. - /// - /// This function returns a `ReturnValue` which should be resolved. The - /// resulting `Value` is unimportant and should be discarded. fn set_property_local( self, reciever: Object<'gc>, @@ -89,7 +85,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error>; + ) -> Result<(), Error>; /// Set a property by it's QName. fn set_property( @@ -107,10 +103,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy } if self.has_own_virtual_setter(name) { - self.set_property_local(reciever, name, value, avm, context)? - .resolve(avm, context)?; - - return Ok(()); + return self.set_property_local(reciever, name, value, avm, context); } let mut proto = self.proto(); @@ -125,17 +118,10 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy proto = my_proto.proto(); } - reciever - .set_property_local(reciever, name, value, avm, context)? - .resolve(avm, context)?; - - Ok(()) + reciever.set_property_local(reciever, name, value, avm, context) } /// Init a property on this specific object. - /// - /// This function returns a `ReturnValue` which should be resolved. The - /// resulting `Value` is unimportant and should be discarded. fn init_property_local( self, reciever: Object<'gc>, @@ -143,7 +129,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error>; + ) -> Result<(), Error>; /// Init a property by it's QName. fn init_property( @@ -161,10 +147,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy } if self.has_own_virtual_setter(name) { - self.init_property_local(reciever, name, value, avm, context)? - .resolve(avm, context)?; - - return Ok(()); + return self.init_property_local(reciever, name, value, avm, context); } let mut proto = self.proto(); @@ -179,11 +162,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy proto = my_proto.proto(); } - reciever - .init_property_local(reciever, name, value, avm, context)? - .resolve(avm, context)?; - - Ok(()) + reciever.init_property_local(reciever, name, value, avm, context) } /// Retrieve a slot by it's index. @@ -521,7 +500,6 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy )?; let super_class: Result, Error> = self .get_property(reciever, &super_name, avm, context)? - .resolve(avm, context)? .as_object() .map_err(|_e| { format!("Could not resolve superclass {:?}", super_name.local_name()).into() @@ -582,7 +560,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy _avm: &mut Avm2<'gc>, _context: &mut UpdateContext<'_, 'gc, '_>, _base_proto: Option>, - ) -> Result, Error> { + ) -> Result, Error> { Err("Object is not callable".into()) } diff --git a/core/src/avm2/scope.rs b/core/src/avm2/scope.rs index e9c751ee9..025946b17 100644 --- a/core/src/avm2/scope.rs +++ b/core/src/avm2/scope.rs @@ -127,12 +127,11 @@ impl<'gc> Scope<'gc> { ) -> Result>, Error> { if let Some(qname) = self.locals().resolve_multiname(name)? { if self.locals().has_property(&qname)? { - return Ok(Some(self.values.get_property( - self.values, - &qname, - avm, - context, - )?)); + return Ok(Some( + self.values + .get_property(self.values, &qname, avm, context)? + .into(), + )); } } diff --git a/core/src/avm2/script_object.rs b/core/src/avm2/script_object.rs index 3eef73463..127869b6a 100644 --- a/core/src/avm2/script_object.rs +++ b/core/src/avm2/script_object.rs @@ -76,7 +76,7 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> { name: &QName, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { + ) -> Result, Error> { self.0 .read() .get_property_local(reciever, name, avm, context) @@ -89,10 +89,15 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> { value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { - self.0 + ) -> Result<(), Error> { + let rv = self + .0 .write(context.gc_context) - .set_property_local(reciever, name, value, avm, context) + .set_property_local(reciever, name, value, avm, context)?; + + rv.resolve(avm, context)?; + + Ok(()) } fn init_property_local( @@ -102,10 +107,15 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> { value: Value<'gc>, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { - self.0 + ) -> Result<(), Error> { + let rv = self + .0 .write(context.gc_context) - .init_property_local(reciever, name, value, avm, context) + .init_property_local(reciever, name, value, avm, context)?; + + rv.resolve(avm, context)?; + + Ok(()) } fn is_property_overwritable(self, gc_context: MutationContext<'gc, '_>, name: &QName) -> bool { @@ -417,7 +427,7 @@ impl<'gc> ScriptObjectData<'gc> { name: &QName, avm: &mut Avm2<'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error> { + ) -> Result, Error> { let prop = self.values.get(name); if let Some(prop) = prop { @@ -428,9 +438,10 @@ impl<'gc> ScriptObjectData<'gc> { avm.current_stack_frame() .and_then(|sf| sf.read().base_proto()) .or(self.proto), - ) + )? + .resolve(avm, context) } else { - Ok(Value::Undefined.into()) + Ok(Value::Undefined) } }