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.
This commit is contained in:
David Wendt 2020-06-22 20:56:18 -04:00
parent 3362ec09e8
commit 4cd30455de
6 changed files with 82 additions and 110 deletions

View File

@ -641,10 +641,9 @@ impl<'gc> Avm2<'gc> {
let receiver = self.pop().as_object().ok(); let receiver = self.pop().as_object().ok();
let function = self.pop().as_object()?; let function = self.pop().as_object()?;
let base_proto = receiver.and_then(|r| r.proto()); let base_proto = receiver.and_then(|r| r.proto());
let value = function.call(receiver, &args, self, context, base_proto)?;
function self.push(value);
.call(receiver, &args, self, context, base_proto)?
.push(self);
Ok(()) Ok(())
} }
@ -661,10 +660,9 @@ impl<'gc> Avm2<'gc> {
.get_method(index.0) .get_method(index.0)
.ok_or_else(|| format!("Object method {} does not exist", index.0).into()); .ok_or_else(|| format!("Object method {} does not exist", index.0).into());
let base_proto = receiver.proto(); let base_proto = receiver.proto();
let value = function?.call(Some(receiver), &args, self, context, base_proto)?;
function? self.push(value);
.call(Some(receiver), &args, self, context, base_proto)?
.push(self);
Ok(()) Ok(())
} }
@ -685,12 +683,10 @@ impl<'gc> Avm2<'gc> {
let base_proto = receiver.get_base_proto(&name)?; let base_proto = receiver.get_base_proto(&name)?;
let function = receiver let function = receiver
.get_property(receiver, &name, self, context)? .get_property(receiver, &name, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
let value = function.call(Some(receiver), &args, self, context, base_proto)?;
function self.push(value);
.call(Some(receiver), &args, self, context, base_proto)?
.push(self);
Ok(()) Ok(())
} }
@ -709,10 +705,10 @@ impl<'gc> Avm2<'gc> {
.ok_or_else(|| format!("Could not find method {:?}", multiname.local_name()).into()); .ok_or_else(|| format!("Could not find method {:?}", multiname.local_name()).into());
let function = receiver let function = receiver
.get_property(receiver, &name?, self, context)? .get_property(receiver, &name?, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
let value = function.call(None, &args, self, context, None)?;
function.call(None, &args, self, context, None)?.push(self); self.push(value);
Ok(()) Ok(())
} }
@ -733,12 +729,9 @@ impl<'gc> Avm2<'gc> {
let base_proto = receiver.get_base_proto(&name)?; let base_proto = receiver.get_base_proto(&name)?;
let function = receiver let function = receiver
.get_property(receiver, &name, self, context)? .get_property(receiver, &name, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
function function.call(Some(receiver), &args, self, context, base_proto)?;
.call(Some(receiver), &args, self, context, base_proto)?
.resolve(self, context)?;
Ok(()) Ok(())
} }
@ -760,10 +753,9 @@ impl<'gc> Avm2<'gc> {
self.system_prototypes.function, self.system_prototypes.function,
None, None,
); );
let value = function.call(Some(receiver), &args, self, context, receiver.proto())?;
function self.push(value);
.call(Some(receiver), &args, self, context, receiver.proto())?
.push(self);
Ok(()) Ok(())
} }
@ -796,12 +788,11 @@ impl<'gc> Avm2<'gc> {
let function = base let function = base
.get_property(receiver, &name?, self, context)? .get_property(receiver, &name?, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
function let value = function.call(Some(receiver), &args, self, context, Some(base_proto))?;
.call(Some(receiver), &args, self, context, Some(base_proto))?
.push(self); self.push(value);
Ok(()) Ok(())
} }
@ -834,12 +825,9 @@ impl<'gc> Avm2<'gc> {
let function = base let function = base
.get_property(receiver, &name?, self, context)? .get_property(receiver, &name?, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
function function.call(Some(receiver), &args, self, context, Some(base_proto))?;
.call(Some(receiver), &args, self, context, Some(base_proto))?
.resolve(self, context)?;
Ok(()) Ok(())
} }
@ -866,9 +854,7 @@ impl<'gc> Avm2<'gc> {
format!("Could not resolve property {:?}", multiname.local_name()).into() format!("Could not resolve property {:?}", multiname.local_name()).into()
}); });
let value = object let value = object.get_property(object, &name?, self, context)?;
.get_property(object, &name?, self, context)?
.resolve(self, context)?;
self.push(value); self.push(value);
Ok(()) Ok(())
@ -960,9 +946,7 @@ impl<'gc> Avm2<'gc> {
.into() .into()
}); });
let value = base let value = base.get_property(object, &name?, self, context)?;
.get_property(object, &name?, self, context)?
.resolve(self, context)?;
self.push(value); self.push(value);
@ -1191,12 +1175,10 @@ impl<'gc> Avm2<'gc> {
self, self,
context, context,
)? )?
.resolve(self, context)?
.as_object()?; .as_object()?;
let object = proto.construct(self, context, &args)?; let object = proto.construct(self, context, &args)?;
ctor.call(Some(object), &args, self, context, object.proto())? ctor.call(Some(object), &args, self, context, object.proto())?;
.resolve(self, context)?;
self.push(object); self.push(object);
@ -1219,7 +1201,6 @@ impl<'gc> Avm2<'gc> {
}); });
let mut ctor = source let mut ctor = source
.get_property(source, &ctor_name?, self, context)? .get_property(source, &ctor_name?, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
let proto = ctor let proto = ctor
.get_property( .get_property(
@ -1228,12 +1209,10 @@ impl<'gc> Avm2<'gc> {
self, self,
context, context,
)? )?
.resolve(self, context)?
.as_object()?; .as_object()?;
let object = proto.construct(self, context, &args)?; let object = proto.construct(self, context, &args)?;
ctor.call(Some(object), &args, self, context, Some(proto))? ctor.call(Some(object), &args, self, context, Some(proto))?;
.resolve(self, context)?;
self.push(object); self.push(object);
@ -1263,12 +1242,9 @@ impl<'gc> Avm2<'gc> {
let function = base_proto let function = base_proto
.get_property(receiver, &name, self, context)? .get_property(receiver, &name, self, context)?
.resolve(self, context)?
.as_object()?; .as_object()?;
function function.call(Some(receiver), &args, self, context, Some(base_proto))?;
.call(Some(receiver), &args, self, context, Some(base_proto))?
.resolve(self, context)?;
Ok(()) Ok(())
} }
@ -1345,9 +1321,7 @@ impl<'gc> Avm2<'gc> {
let (new_class, class_init) = let (new_class, class_init) =
FunctionObject::from_abc_class(self, context, class_entry, base_class, scope)?; FunctionObject::from_abc_class(self, context, class_entry, base_class, scope)?;
class_init class_init.call(Some(new_class), &[], self, context, None)?;
.call(Some(new_class), &[], self, context, None)?
.resolve(self, context)?;
self.push(new_class); self.push(new_class);
@ -1500,9 +1474,7 @@ impl<'gc> Avm2<'gc> {
let name = object.get_enumerant_name(cur_index as u32); let name = object.get_enumerant_name(cur_index as u32);
let value = if let Some(name) = name { let value = if let Some(name) = name {
object object.get_property(object, &name, self, context)?
.get_property(object, &name, self, context)?
.resolve(self, context)?
} else { } else {
Value::Undefined Value::Undefined
}; };

View File

@ -284,7 +284,6 @@ impl<'gc> FunctionObject<'gc> {
avm, avm,
context, context,
)? )?
.resolve(avm, context)?
.as_object() .as_object()
.map_err(|_| { .map_err(|_| {
let super_name = QName::from_abc_multiname( let super_name = QName::from_abc_multiname(
@ -443,7 +442,7 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
name: &QName, name: &QName,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
self.0 self.0
.read() .read()
.base .base
@ -457,11 +456,16 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<(), Error> {
self.0 let rv = self
.0
.write(context.gc_context) .write(context.gc_context)
.base .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( fn init_property_local(
@ -471,11 +475,16 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<(), Error> {
self.0 let rv = self
.0
.write(context.gc_context) .write(context.gc_context)
.base .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 { 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>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
base_proto: Option<Object<'gc>>, base_proto: Option<Object<'gc>>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
if let Some(exec) = &self.0.read().exec { 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 { } else {
Err("Not a callable function!".into()) Err("Not a callable function!".into())
} }

View File

@ -32,9 +32,11 @@ fn call<'gc>(
if let Some(func) = func { if let Some(func) = func {
if args.len() > 1 { 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 { } else {
func.call(this, &[], avm, context, base_proto) Ok(func.call(this, &[], avm, context, base_proto)?.into())
} }
} else { } else {
Err("Not a callable function".into()) Err("Not a callable function".into())

View File

@ -2,7 +2,6 @@
use crate::avm2::function::{Avm2ClassEntry, Avm2MethodEntry, Executable, FunctionObject}; use crate::avm2::function::{Avm2ClassEntry, Avm2MethodEntry, Executable, FunctionObject};
use crate::avm2::names::{Multiname, Namespace, QName}; use crate::avm2::names::{Multiname, Namespace, QName};
use crate::avm2::return_value::ReturnValue;
use crate::avm2::scope::Scope; use crate::avm2::scope::Scope;
use crate::avm2::script_object::ScriptObject; use crate::avm2::script_object::ScriptObject;
use crate::avm2::value::{abc_default_value, Value}; use crate::avm2::value::{abc_default_value, Value};
@ -33,7 +32,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
name: &QName, name: &QName,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error>; ) -> Result<Value<'gc>, Error>;
/// Retrieve a property by it's QName. /// Retrieve a property by it's QName.
fn get_property( fn get_property(
@ -42,7 +41,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
name: &QName, name: &QName,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
if !self.has_instantiated_property(name) { if !self.has_instantiated_property(name) {
for abc_trait in self.get_trait(name)? { for abc_trait in self.get_trait(name)? {
self.install_trait(avm, context, &abc_trait, reciever)?; self.install_trait(avm, context, &abc_trait, reciever)?;
@ -59,7 +58,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
return proto.get_property(reciever, name, avm, context); 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. /// Retrieve the base prototype that a particular QName trait is defined in.
@ -79,9 +78,6 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
} }
/// Set a property on this specific object. /// 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( fn set_property_local(
self, self,
reciever: Object<'gc>, reciever: Object<'gc>,
@ -89,7 +85,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error>; ) -> Result<(), Error>;
/// Set a property by it's QName. /// Set a property by it's QName.
fn set_property( fn set_property(
@ -107,10 +103,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
} }
if self.has_own_virtual_setter(name) { if self.has_own_virtual_setter(name) {
self.set_property_local(reciever, name, value, avm, context)? return self.set_property_local(reciever, name, value, avm, context);
.resolve(avm, context)?;
return Ok(());
} }
let mut proto = self.proto(); let mut proto = self.proto();
@ -125,17 +118,10 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
proto = my_proto.proto(); proto = my_proto.proto();
} }
reciever reciever.set_property_local(reciever, name, value, avm, context)
.set_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
Ok(())
} }
/// Init a property on this specific object. /// 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( fn init_property_local(
self, self,
reciever: Object<'gc>, reciever: Object<'gc>,
@ -143,7 +129,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error>; ) -> Result<(), Error>;
/// Init a property by it's QName. /// Init a property by it's QName.
fn init_property( fn init_property(
@ -161,10 +147,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
} }
if self.has_own_virtual_setter(name) { if self.has_own_virtual_setter(name) {
self.init_property_local(reciever, name, value, avm, context)? return self.init_property_local(reciever, name, value, avm, context);
.resolve(avm, context)?;
return Ok(());
} }
let mut proto = self.proto(); let mut proto = self.proto();
@ -179,11 +162,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
proto = my_proto.proto(); proto = my_proto.proto();
} }
reciever reciever.init_property_local(reciever, name, value, avm, context)
.init_property_local(reciever, name, value, avm, context)?
.resolve(avm, context)?;
Ok(())
} }
/// Retrieve a slot by it's index. /// Retrieve a slot by it's index.
@ -521,7 +500,6 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
)?; )?;
let super_class: Result<Object<'gc>, Error> = self let super_class: Result<Object<'gc>, Error> = self
.get_property(reciever, &super_name, avm, context)? .get_property(reciever, &super_name, avm, context)?
.resolve(avm, context)?
.as_object() .as_object()
.map_err(|_e| { .map_err(|_e| {
format!("Could not resolve superclass {:?}", super_name.local_name()).into() format!("Could not resolve superclass {:?}", super_name.local_name()).into()
@ -582,7 +560,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
_avm: &mut Avm2<'gc>, _avm: &mut Avm2<'gc>,
_context: &mut UpdateContext<'_, 'gc, '_>, _context: &mut UpdateContext<'_, 'gc, '_>,
_base_proto: Option<Object<'gc>>, _base_proto: Option<Object<'gc>>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
Err("Object is not callable".into()) Err("Object is not callable".into())
} }

View File

@ -127,12 +127,11 @@ impl<'gc> Scope<'gc> {
) -> Result<Option<ReturnValue<'gc>>, Error> { ) -> Result<Option<ReturnValue<'gc>>, Error> {
if let Some(qname) = self.locals().resolve_multiname(name)? { if let Some(qname) = self.locals().resolve_multiname(name)? {
if self.locals().has_property(&qname)? { if self.locals().has_property(&qname)? {
return Ok(Some(self.values.get_property( return Ok(Some(
self.values, self.values
&qname, .get_property(self.values, &qname, avm, context)?
avm, .into(),
context, ));
)?));
} }
} }

View File

@ -76,7 +76,7 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
name: &QName, name: &QName,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
self.0 self.0
.read() .read()
.get_property_local(reciever, name, avm, context) .get_property_local(reciever, name, avm, context)
@ -89,10 +89,15 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<(), Error> {
self.0 let rv = self
.0
.write(context.gc_context) .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( fn init_property_local(
@ -102,10 +107,15 @@ impl<'gc> TObject<'gc> for ScriptObject<'gc> {
value: Value<'gc>, value: Value<'gc>,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<(), Error> {
self.0 let rv = self
.0
.write(context.gc_context) .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 { fn is_property_overwritable(self, gc_context: MutationContext<'gc, '_>, name: &QName) -> bool {
@ -417,7 +427,7 @@ impl<'gc> ScriptObjectData<'gc> {
name: &QName, name: &QName,
avm: &mut Avm2<'gc>, avm: &mut Avm2<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>, context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> { ) -> Result<Value<'gc>, Error> {
let prop = self.values.get(name); let prop = self.values.get(name);
if let Some(prop) = prop { if let Some(prop) = prop {
@ -428,9 +438,10 @@ impl<'gc> ScriptObjectData<'gc> {
avm.current_stack_frame() avm.current_stack_frame()
.and_then(|sf| sf.read().base_proto()) .and_then(|sf| sf.read().base_proto())
.or(self.proto), .or(self.proto),
) )?
.resolve(avm, context)
} else { } else {
Ok(Value::Undefined.into()) Ok(Value::Undefined)
} }
} }