From 4906c5a3f17150cd7a238510c6ed6168d8f9aae6 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Tue, 28 Jul 2020 21:31:58 -0400 Subject: [PATCH] Remove uses of `as_string` in various places. These include: * Name resolution in `newobject` * All runtime & late-bound multinames * `Object.hasOwnProperty` * `Object.propertyIsEnumerable` * `Object.setPropertyIsEnumerable` --- core/src/avm2/activation.rs | 37 ++++++++++++++++----------------- core/src/avm2/globals/object.rs | 10 ++++----- core/src/avm2/names.rs | 36 ++++++++++++++++++++------------ core/src/avm2/value.rs | 10 --------- 4 files changed, 46 insertions(+), 47 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index 300385cdc..416b8617e 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -343,9 +343,8 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { &mut self, method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, - mc: MutationContext<'gc, '_>, ) -> Result, Error> { - Multiname::from_abc_multiname(method.translation_unit(), index, self.context.avm2, mc) + Multiname::from_abc_multiname(method.translation_unit(), index, self) } /// Retrieve a static, or non-runtime, multiname from the current constant @@ -706,7 +705,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut receiver = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = receiver .resolve_multiname(&multiname)? @@ -730,7 +729,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut receiver = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = receiver .resolve_multiname(&multiname)? @@ -752,7 +751,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut receiver = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = receiver .resolve_multiname(&multiname)? @@ -799,7 +798,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let receiver = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = receiver .resolve_multiname(&multiname)? @@ -831,7 +830,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let receiver = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = receiver .resolve_multiname(&multiname)? @@ -869,7 +868,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error> { - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut object = self.context.avm2.pop().coerce_to_object(self)?; let name: Result = object.resolve_multiname(&multiname)?.ok_or_else(|| { @@ -888,7 +887,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { index: Index, ) -> Result, Error> { let value = self.context.avm2.pop(); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut object = self.context.avm2.pop().coerce_to_object(self)?; if let Some(name) = object.resolve_multiname(&multiname)? { @@ -912,7 +911,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { index: Index, ) -> Result, Error> { let value = self.context.avm2.pop(); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut object = self.context.avm2.pop().coerce_to_object(self)?; if let Some(name) = object.resolve_multiname(&multiname)? { @@ -935,7 +934,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error> { - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let object = self.context.avm2.pop().coerce_to_object(self)?; if let Some(name) = object.resolve_multiname(&multiname)? { @@ -954,7 +953,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error> { - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let object = self.context.avm2.pop().coerce_to_object(self)?; let base_proto: Result, Error> = self .base_proto() @@ -984,7 +983,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { index: Index, ) -> Result, Error> { let value = self.context.avm2.pop(); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let object = self.context.avm2.pop().coerce_to_object(self)?; let base_proto: Result, Error> = self .base_proto() @@ -1081,8 +1080,8 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error> { - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; - avm_debug!(self.avm2(), "Resolving {:?}", multiname); + let multiname = self.pool_multiname(method, index)?; + avm_debug!(self.context.avm2, "Resolving {:?}", multiname); let result = if let Some(scope) = self.scope() { scope.read().find(&multiname, self)? } else { @@ -1101,8 +1100,8 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { method: Gc<'gc, BytecodeMethod<'gc>>, index: Index, ) -> Result, Error> { - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; - avm_debug!(self.avm2(), "Resolving {:?}", multiname); + let multiname = self.pool_multiname(method, index)?; + avm_debug!(self.context.avm2, "Resolving {:?}", multiname); let found: Result, Error> = if let Some(scope) = self.scope() { scope.read().find(&multiname, self)? } else { @@ -1202,7 +1201,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { arg_count: u32, ) -> Result, Error> { let args = self.context.avm2.pop_args(arg_count); - let multiname = self.pool_multiname(method, index, self.context.gc_context)?; + let multiname = self.pool_multiname(method, index)?; let mut source = self.context.avm2.pop().coerce_to_object(self)?; let ctor_name: Result = @@ -1269,7 +1268,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> { object.set_property( object, - &QName::new(Namespace::public_namespace(), name.as_string()?), + &QName::new(Namespace::public_namespace(), name.coerce_to_string(self)?), value, self, )?; diff --git a/core/src/avm2/globals/object.rs b/core/src/avm2/globals/object.rs index fab2afc37..9ae392100 100644 --- a/core/src/avm2/globals/object.rs +++ b/core/src/avm2/globals/object.rs @@ -52,14 +52,14 @@ fn value_of<'gc>( /// `Object.prototype.hasOwnProperty` pub fn has_own_property<'gc>( - _activation: &mut Activation<'_, 'gc, '_>, + activation: &mut Activation<'_, 'gc, '_>, this: Option>, args: &[Value<'gc>], ) -> Result, Error> { let this: Result, Error> = this.ok_or_else(|| "No valid this parameter".into()); let this = this?; let name: Result<&Value<'gc>, Error> = args.get(0).ok_or_else(|| "No name specified".into()); - let name = name?.as_string()?; + let name = name?.coerce_to_string(activation)?; if let Some(ns) = this.resolve_any(name)? { if !ns.is_private() { @@ -95,14 +95,14 @@ pub fn is_prototype_of<'gc>( /// `Object.prototype.propertyIsEnumerable` pub fn property_is_enumerable<'gc>( - _activation: &mut Activation<'_, 'gc, '_>, + activation: &mut Activation<'_, 'gc, '_>, this: Option>, args: &[Value<'gc>], ) -> Result, Error> { let this: Result, Error> = this.ok_or_else(|| "No valid this parameter".into()); let this = this?; let name: Result<&Value<'gc>, Error> = args.get(0).ok_or_else(|| "No name specified".into()); - let name = name?.as_string()?; + let name = name?.coerce_to_string(activation)?; if let Some(ns) = this.resolve_any(name)? { if !ns.is_private() { @@ -123,7 +123,7 @@ pub fn set_property_is_enumerable<'gc>( let this: Result, Error> = this.ok_or_else(|| "No valid this parameter".into()); let this = this?; let name: Result<&Value<'gc>, Error> = args.get(0).ok_or_else(|| "No name specified".into()); - let name = name?.as_string()?; + let name = name?.coerce_to_string(activation)?; if let Some(Value::Bool(is_enum)) = args.get(1) { if let Some(ns) = this.resolve_any(name)? { diff --git a/core/src/avm2/names.rs b/core/src/avm2/names.rs index 8b9855646..69d7a7bef 100644 --- a/core/src/avm2/names.rs +++ b/core/src/avm2/names.rs @@ -1,8 +1,9 @@ //! AVM2 names & namespacing +use crate::avm2::activation::Activation; use crate::avm2::script::TranslationUnit; use crate::avm2::string::AvmString; -use crate::avm2::{Avm2, Error}; +use crate::avm2::Error; use gc_arena::{Collect, MutationContext}; use swf::avm2::types::{ Index, Multiname as AbcMultiname, Namespace as AbcNamespace, NamespaceSet as AbcNamespaceSet, @@ -222,8 +223,7 @@ impl<'gc> Multiname<'gc> { pub fn from_abc_multiname( translation_unit: TranslationUnit<'gc>, multiname_index: Index, - avm: &mut Avm2<'gc>, - mc: MutationContext<'gc, '_>, + activation: &mut Activation<'_, 'gc, '_>, ) -> Result { let actual_index: Result = (multiname_index.0 as usize) .checked_sub(1) @@ -242,21 +242,23 @@ impl<'gc> Multiname<'gc> { ns: vec![Namespace::from_abc_namespace( translation_unit, namespace.clone(), - mc, + activation.context.gc_context, )?], - name: translation_unit.pool_string_option(name.0, mc)?, + name: translation_unit + .pool_string_option(name.0, activation.context.gc_context)?, } } AbcMultiname::RTQName { name } | AbcMultiname::RTQNameA { name } => { - let ns = avm.pop().as_namespace()?.clone(); + let ns = activation.avm2().pop().as_namespace()?.clone(); Self { ns: vec![ns], - name: translation_unit.pool_string_option(name.0, mc)?, + name: translation_unit + .pool_string_option(name.0, activation.context.gc_context)?, } } AbcMultiname::RTQNameL | AbcMultiname::RTQNameLA => { - let ns = avm.pop().as_namespace()?.clone(); - let name = avm.pop().as_string()?; + let ns = activation.avm2().pop().as_namespace()?.clone(); + let name = activation.avm2().pop().coerce_to_string(activation)?; Self { ns: vec![ns], name: Some(name), @@ -270,14 +272,22 @@ impl<'gc> Multiname<'gc> { namespace_set, name, } => Self { - ns: Self::abc_namespace_set(translation_unit, namespace_set.clone(), mc)?, - name: translation_unit.pool_string_option(name.0, mc)?, + ns: Self::abc_namespace_set( + translation_unit, + namespace_set.clone(), + activation.context.gc_context, + )?, + name: translation_unit.pool_string_option(name.0, activation.context.gc_context)?, }, AbcMultiname::MultinameL { namespace_set } | AbcMultiname::MultinameLA { namespace_set } => { - let name = avm.pop().as_string()?; + let name = activation.avm2().pop().coerce_to_string(activation)?; Self { - ns: Self::abc_namespace_set(translation_unit, namespace_set.clone(), mc)?, + ns: Self::abc_namespace_set( + translation_unit, + namespace_set.clone(), + activation.context.gc_context, + )?, name: Some(name), } } diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs index f4b613d7a..9ef429855 100644 --- a/core/src/avm2/value.rs +++ b/core/src/avm2/value.rs @@ -206,16 +206,6 @@ pub fn abc_default_value<'gc>( } impl<'gc> Value<'gc> { - /// Demand a string value, erroring out if one is not found. - /// - /// TODO: This should be replaced with `coerce_string` where possible. - pub fn as_string(&self) -> Result, Error> { - match self { - Value::String(s) => Ok(*s), - _ => Err(format!("Expected String, found {:?}", self).into()), - } - } - pub fn as_number(&self) -> Result { match self { Value::Number(f) => Ok(*f),