From d8f043fa1a4231f024315a0890241c29e4285ffc Mon Sep 17 00:00:00 2001 From: Nathan Adams Date: Mon, 13 Jul 2020 01:29:04 +0200 Subject: [PATCH] avm1: Value::coerce_to_string returns an Avm1String, as it can avoid a clone-and-reallocate --- core/src/avm1.rs | 2 +- core/src/avm1/activation.rs | 8 +++--- core/src/avm1/debug.rs | 2 +- core/src/avm1/globals.rs | 2 +- core/src/avm1/globals/array.rs | 8 +++--- core/src/avm1/globals/movie_clip.rs | 5 ++-- core/src/avm1/globals/object.rs | 12 ++++++--- core/src/avm1/globals/string.rs | 37 ++++------------------------ core/src/avm1/globals/text_field.rs | 6 ++--- core/src/avm1/globals/xml.rs | 3 +-- core/src/avm1/object/stage_object.rs | 2 +- core/src/avm1/string.rs | 22 +++++++++++++++++ core/src/avm1/value.rs | 23 +++++++++-------- core/src/display_object/edit_text.rs | 2 +- 14 files changed, 69 insertions(+), 65 deletions(-) diff --git a/core/src/avm1.rs b/core/src/avm1.rs index bb1820080..bb7c7ed4b 100644 --- a/core/src/avm1.rs +++ b/core/src/avm1.rs @@ -394,7 +394,7 @@ pub fn root_error_handler<'gc>( if let Error::ThrownValue(error) = &error { let string = error .coerce_to_string(activation, context) - .unwrap_or_else(|_| Cow::Borrowed("undefined")); + .unwrap_or_else(|_| "undefined".into()); log::info!(target: "avm_trace", "{}", string); } else { log::error!("{}", error); diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index 948c9c11f..39ffff0e2 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -1299,7 +1299,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> { .coerce_to_object(self, context); let (url, opts) = self.locals_into_request_options( context, - url, + Cow::Borrowed(&url), NavigationMethod::from_send_vars_method(swf_method), ); let fetch = context.navigator.fetch(&url, opts); @@ -1317,7 +1317,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> { if let Some(clip_target) = clip_target { let (url, opts) = self.locals_into_request_options( context, - url, + Cow::Borrowed(&url), NavigationMethod::from_send_vars_method(swf_method), ); let fetch = context.navigator.fetch(&url, opts); @@ -2277,7 +2277,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> { // trace always prints "undefined" even though SWF6 and below normally // coerce undefined to "". let out = if val == Value::Undefined { - Cow::Borrowed("undefined") + "undefined".into() } else { val.coerce_to_string(self, context)? }; @@ -2480,7 +2480,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> { v.ok() .unwrap_or_else(|| Value::Undefined) .coerce_to_string(self, context) - .unwrap_or_else(|_| Cow::Borrowed("undefined")) + .unwrap_or_else(|_| "undefined".into()) .to_string(), ); } diff --git a/core/src/avm1/debug.rs b/core/src/avm1/debug.rs index fccbd3c2a..a01392cca 100644 --- a/core/src/avm1/debug.rs +++ b/core/src/avm1/debug.rs @@ -198,7 +198,7 @@ mod tests { use super::*; use crate::avm1::error::Error; use crate::avm1::test_utils::with_avm; - use crate::avm1::{Avm1String, ScriptObject}; + use crate::avm1::ScriptObject; #[test] fn dump_undefined() { diff --git a/core/src/avm1/globals.rs b/core/src/avm1/globals.rs index d8c473011..c5d46791b 100644 --- a/core/src/avm1/globals.rs +++ b/core/src/avm1/globals.rs @@ -167,7 +167,7 @@ pub fn create_timer<'a, 'gc>( .get(1) .unwrap_or(&Value::Undefined) .coerce_to_string(activation, context)? - .into_owned(), + .to_string(), }, 2, ), diff --git a/core/src/avm1/globals/array.rs b/core/src/avm1/globals/array.rs index 9b697ff07..1d0f5b14a 100644 --- a/core/src/avm1/globals/array.rs +++ b/core/src/avm1/globals/array.rs @@ -8,7 +8,6 @@ use crate::avm1::property::Attribute; use crate::avm1::{Avm1String, Object, ScriptObject, TObject, UpdateContext, Value}; use enumset::EnumSet; use gc_arena::MutationContext; -use smallvec::alloc::borrow::Cow; use std::cmp::Ordering; // Flags used by `Array.sort` and `sortOn`. @@ -244,7 +243,7 @@ pub fn join<'gc>( let separator = args .get(0) .and_then(|v| v.coerce_to_string(activation, context).ok()) - .unwrap_or_else(|| Cow::Borrowed(",")); + .unwrap_or_else(|| ",".into()); let values: Vec> = this.array(); Ok(Avm1String::new( @@ -253,9 +252,10 @@ pub fn join<'gc>( .iter() .map(|v| { v.coerce_to_string(activation, context) - .unwrap_or_else(|_| Cow::Borrowed("undefined")) + .unwrap_or_else(|_| "undefined".into()) + .to_string() }) - .collect::>>() + .collect::>() .join(&separator), ) .into()) diff --git a/core/src/avm1/globals/movie_clip.rs b/core/src/avm1/globals/movie_clip.rs index 0a141bc4f..8daa850c2 100644 --- a/core/src/avm1/globals/movie_clip.rs +++ b/core/src/avm1/globals/movie_clip.rs @@ -12,6 +12,7 @@ use crate::prelude::*; use crate::shape_utils::DrawCommand; use crate::tag_utils::SwfSlice; use gc_arena::MutationContext; +use std::borrow::Cow; use swf::{ FillStyle, Gradient, GradientInterpolation, GradientRecord, GradientSpread, LineCapStyle, LineJoinStyle, LineStyle, Twips, @@ -1047,7 +1048,7 @@ fn load_movie<'gc>( let url = url_val.coerce_to_string(activation, context)?; let method = args.get(1).cloned().unwrap_or(Value::Undefined); let method = NavigationMethod::from_method_str(&method.coerce_to_string(activation, context)?); - let (url, opts) = activation.locals_into_request_options(context, url, method); + let (url, opts) = activation.locals_into_request_options(context, Cow::Borrowed(&url), method); let fetch = context.navigator.fetch(&url, opts); let process = context.load_manager.load_movie_into_clip( context.player.clone().unwrap(), @@ -1071,7 +1072,7 @@ fn load_variables<'gc>( let url = url_val.coerce_to_string(activation, context)?; let method = args.get(1).cloned().unwrap_or(Value::Undefined); let method = NavigationMethod::from_method_str(&method.coerce_to_string(activation, context)?); - let (url, opts) = activation.locals_into_request_options(context, url, method); + let (url, opts) = activation.locals_into_request_options(context, Cow::Borrowed(&url), method); let fetch = context.navigator.fetch(&url, opts); let target = target.object().coerce_to_object(activation, context); let process = diff --git a/core/src/avm1/globals/object.rs b/core/src/avm1/globals/object.rs index dbc3bcfd6..c8bcc3da3 100644 --- a/core/src/avm1/globals/object.rs +++ b/core/src/avm1/globals/object.rs @@ -29,7 +29,7 @@ pub fn add_property<'gc>( let name = args .get(0) .and_then(|v| v.coerce_to_string(activation, context).ok()) - .unwrap_or_else(|| Cow::Borrowed("undefined")); + .unwrap_or_else(|| "undefined".into()); let getter = args.get(1).unwrap_or(&Value::Undefined); let setter = args.get(2).unwrap_or(&Value::Undefined); @@ -188,7 +188,13 @@ fn watch<'gc>( }; let user_data = args.get(2).cloned().unwrap_or(Value::Undefined); - this.set_watcher(activation, context.gc_context, name, callback, user_data); + this.set_watcher( + activation, + context.gc_context, + Cow::Borrowed(&name), + callback, + user_data, + ); Ok(true.into()) } @@ -206,7 +212,7 @@ fn unwatch<'gc>( return Ok(false.into()); }; - let result = this.remove_watcher(activation, context.gc_context, name); + let result = this.remove_watcher(activation, context.gc_context, Cow::Borrowed(&name)); Ok(result.into()) } diff --git a/core/src/avm1/globals/string.rs b/core/src/avm1/globals/string.rs index f2a5615d5..2d9136aa8 100644 --- a/core/src/avm1/globals/string.rs +++ b/core/src/avm1/globals/string.rs @@ -20,10 +20,7 @@ pub fn string<'gc>( ) -> Result, Error<'gc>> { let value = match args.get(0).cloned() { Some(Value::String(s)) => s, - Some(v) => Avm1String::new( - ac.gc_context, - v.coerce_to_string(activation, ac)?.to_string(), - ), + Some(v) => v.coerce_to_string(activation, ac)?, _ => Avm1String::new(ac.gc_context, String::new()), }; @@ -182,13 +179,7 @@ fn char_at<'gc>( // TODO: Will return REPLACEMENT_CHAR if this indexes a character outside the BMP, losing info about the surrogate. // When we improve our string representation, the unpaired surrogate should be returned. let this_val = Value::from(this); - let string = match this_val { - Value::String(string) => string, - other => Avm1String::new( - context.gc_context, - other.coerce_to_string(activation, context)?.to_string(), - ), - }; + let string = this_val.coerce_to_string(activation, context)?; let i = args .get(0) .unwrap_or(&Value::Undefined) @@ -406,13 +397,7 @@ fn split<'gc>( args: &[Value<'gc>], ) -> Result, Error<'gc>> { let this_val = Value::from(this); - let this = match this_val { - Value::String(string) => string, - other => Avm1String::new( - context.gc_context, - other.coerce_to_string(activation, context)?.to_string(), - ), - }; + let this = this_val.coerce_to_string(activation, context)?; let delimiter_val = args.get(0).unwrap_or(&Value::Undefined); let delimiter = delimiter_val.coerce_to_string(activation, context)?; let limit = match args.get(1) { @@ -454,13 +439,7 @@ fn substr<'gc>( } let this_val = Value::from(this); - let this = match this_val { - Value::String(string) => string, - other => Avm1String::new( - context.gc_context, - other.coerce_to_string(activation, context)?.to_string(), - ), - }; + let this = this_val.coerce_to_string(activation, context)?; let this_len = this.encode_utf16().count(); let start_index = string_wrapping_index( args.get(0).unwrap().coerce_to_i32(activation, context)?, @@ -487,13 +466,7 @@ fn substring<'gc>( } let this_val = Value::from(this); - let this = match this_val { - Value::String(string) => string, - other => Avm1String::new( - context.gc_context, - other.coerce_to_string(activation, context)?.to_string(), - ), - }; + let this = this_val.coerce_to_string(activation, context)?; let this_len = this.encode_utf16().count(); let mut start_index = string_index( args.get(0).unwrap().coerce_to_i32(activation, context)?, diff --git a/core/src/avm1/globals/text_field.rs b/core/src/avm1/globals/text_field.rs index 1d6e6f8f5..98e0ba977 100644 --- a/core/src/avm1/globals/text_field.rs +++ b/core/src/avm1/globals/text_field.rs @@ -112,7 +112,7 @@ pub fn set_html_text<'gc>( .get(0) .unwrap_or(&Value::Undefined) .coerce_to_string(activation, context)?; - let _ = text_field.set_html_text(text.into_owned(), context); + let _ = text_field.set_html_text(text.to_string(), context); // Changing the htmlText does NOT update variable bindings (does not call EditText::propagate_text_binding). } } @@ -326,7 +326,7 @@ fn set_variable<'gc>( .as_display_object() .and_then(|dobj| dobj.as_edit_text()) { - etext.set_variable(variable.map(|v| v.into_owned()), activation, context); + etext.set_variable(variable.map(|v| v.to_string()), activation, context); } Ok(Value::Undefined) @@ -627,7 +627,7 @@ fn replace_text<'gc>( .cloned() .unwrap_or(Value::Undefined) .coerce_to_string(activation, context)? - .into_owned(); + .to_string(); text_field.replace_text(from as usize, to as usize, &text, context); diff --git a/core/src/avm1/globals/xml.rs b/core/src/avm1/globals/xml.rs index e7d8b9130..049469c74 100644 --- a/core/src/avm1/globals/xml.rs +++ b/core/src/avm1/globals/xml.rs @@ -13,7 +13,6 @@ use crate::xml::{XMLDocument, XMLNode}; use enumset::EnumSet; use gc_arena::MutationContext; use quick_xml::Error as ParseError; -use std::borrow::Cow; pub const XML_NO_ERROR: f64 = 0.0; #[allow(dead_code)] @@ -791,7 +790,7 @@ pub fn xml_parse_xml<'gc>( if let Some(Ok(xmlstring)) = args.get(0).map(|s| s.coerce_to_string(activation, ac)) { xmlstring } else { - Cow::Borrowed("") + "".into() }; if let Some(children) = node.children() { diff --git a/core/src/avm1/object/stage_object.rs b/core/src/avm1/object/stage_object.rs index a88e3b097..aad479f37 100644 --- a/core/src/avm1/object/stage_object.rs +++ b/core/src/avm1/object/stage_object.rs @@ -187,7 +187,7 @@ impl<'gc> TObject<'gc> for StageObject<'gc> { .filter(|binding| binding.variable_name == name) { let _ = binding.text_field.set_html_text( - value.coerce_to_string(activation, context)?.into_owned(), + value.coerce_to_string(activation, context)?.to_string(), context, ); } diff --git a/core/src/avm1/string.rs b/core/src/avm1/string.rs index e8212f5c4..a9b8243fa 100644 --- a/core/src/avm1/string.rs +++ b/core/src/avm1/string.rs @@ -1,4 +1,5 @@ use gc_arena::{Collect, Gc, MutationContext}; +use std::fmt; use std::ops::Deref; #[derive(Debug, Clone, Collect)] @@ -26,6 +27,14 @@ impl<'gc> Avm1String<'gc> { } } +impl Default for Avm1String<'_> { + fn default() -> Self { + Self { + source: Source::Static(""), + } + } +} + impl<'gc> From<&'static str> for Avm1String<'gc> { fn from(str: &'static str) -> Self { Self { @@ -34,6 +43,12 @@ impl<'gc> From<&'static str> for Avm1String<'gc> { } } +impl fmt::Display for Avm1String<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self) + } +} + impl Deref for Avm1String<'_> { type Target = str; @@ -56,6 +71,13 @@ impl AsRef for Avm1String<'_> { } } +impl<'gc> PartialEq> for Avm1String<'gc> { + #[inline] + fn eq(&self, other: &Avm1String<'gc>) -> bool { + PartialEq::eq(self.as_str(), other.as_str()) + } +} + macro_rules! impl_eq { ($lhs:ty, $rhs: ty) => { #[allow(unused_lifetimes)] diff --git a/core/src/avm1/value.rs b/core/src/avm1/value.rs index 7fbda2366..49c07966c 100644 --- a/core/src/avm1/value.rs +++ b/core/src/avm1/value.rs @@ -457,26 +457,29 @@ impl<'gc> Value<'gc> { &'a self, activation: &mut Activation<'_, 'gc>, context: &mut UpdateContext<'_, 'gc, '_>, - ) -> Result, Error<'gc>> { + ) -> Result, Error<'gc>> { Ok(match self { Value::Object(object) => { match object.call_method("toString", &[], activation, context)? { - Value::String(s) => Cow::Owned(s.to_string()), - _ => Cow::Borrowed("[type Object]"), + Value::String(s) => s, + _ => "[type Object]".into(), } } Value::Undefined => { if activation.current_swf_version() >= 7 { - Cow::Borrowed("undefined") + "undefined".into() } else { - Cow::Borrowed("") + "".into() } } - Value::Null => Cow::Borrowed("null"), - Value::Bool(true) => Cow::Borrowed("true"), - Value::Bool(false) => Cow::Borrowed("false"), - Value::Number(v) => f64_to_string(*v), - Value::String(v) => Cow::Borrowed(v), + Value::Null => "null".into(), + Value::Bool(true) => "true".into(), + Value::Bool(false) => "false".into(), + Value::Number(v) => match f64_to_string(*v) { + Cow::Borrowed(str) => str.into(), + Cow::Owned(str) => Avm1String::new(context.gc_context, str), + }, + Value::String(v) => v.to_owned(), }) } diff --git a/core/src/display_object/edit_text.rs b/core/src/display_object/edit_text.rs index 1796238c0..8f4331fbb 100644 --- a/core/src/display_object/edit_text.rs +++ b/core/src/display_object/edit_text.rs @@ -733,7 +733,7 @@ impl<'gc> EditText<'gc> { value .coerce_to_string(activation, context) .unwrap_or_default() - .into_owned(), + .to_string(), context, ); } else {