avm2: Make primitive coercions way cheaper

core/src/avm2/value.rs

@@ -656,21 +656,14 @@ impl<'gc> Value<'gc> {
         match self {
             Value::Object(Object::PrimitiveObject(o)) => o.value_of(activation.context.gc_context),
             Value::Object(o) if hint == Hint::String => {
-                let mut prim = *self;
                 let object = *o;
 
-                if let Value::Object(_) = object.get_public_property("toString", activation)? {
-                    prim = object.call_public_property("toString", &[], activation)?;
-                }
-
+                let prim = object.call_public_property("toString", &[], activation)?;
                 if prim.is_primitive() {
                     return Ok(prim);
                 }
 
-                if let Value::Object(_) = object.get_public_property("valueOf", activation)? {
-                    prim = object.call_public_property("valueOf", &[], activation)?;
-                }
-
+                let prim = object.call_public_property("valueOf", &[], activation)?;
                 if prim.is_primitive() {
                     return Ok(prim);
                 }
@@ -685,21 +678,14 @@ impl<'gc> Value<'gc> {
                 )?))
             }
             Value::Object(o) if hint == Hint::Number => {
-                let mut prim = *self;
                 let object = *o;
 
-                if let Value::Object(_) = object.get_public_property("valueOf", activation)? {
-                    prim = object.call_public_property("valueOf", &[], activation)?;
-                }
-
+                let prim = object.call_public_property("valueOf", &[], activation)?;
                 if prim.is_primitive() {
                     return Ok(prim);
                 }
 
-                if let Value::Object(_) = object.get_public_property("toString", activation)? {
-                    prim = object.call_public_property("toString", &[], activation)?;
-                }
-
+                let prim = object.call_public_property("toString", &[], activation)?;
                 if prim.is_primitive() {
                     return Ok(prim);
                 }

tests/tests/swfs/avm2/coerce_to_primitive_side_effects/Test.as

@@ -0,0 +1,58 @@
+// compiled with mxmlc
+
+import flash.utils.getQualifiedClassName;
+import flash.utils.Proxy;
+import flash.utils.flash_proxy;
+
+package {
+    import flash.display.MovieClip;
+    public class Test extends MovieClip {
+    }
+}
+
+class MyProxy extends Proxy {
+    public var returnValue;
+    public function MyProxy(val) { this.returnValue = val; }
+    flash_proxy override function callProperty(name:*, ...rest):* {
+        trace("call " + name);
+        return this.returnValue;
+    }
+    flash_proxy override function getProperty(name:*):* {
+        trace("get " + name);
+        return undefined;
+    }
+    flash_proxy override function hasProperty(name:*):Boolean {
+        trace("has " + name);
+        return false;
+    }
+}
+
+function test(obj){
+    trace("----------- " + obj.returnValue);
+    var s: String;
+    var i: int;
+    try {
+        s = obj;
+        trace(getQualifiedClassName(s));
+        trace(s);
+    } catch (e) { trace("Error " + e.errorID); }
+
+    try {
+        i = obj;
+        trace(getQualifiedClassName(i));
+        trace(i);
+    } catch (e) { trace("Error " + e.errorID); }
+}
+
+function main(){
+    test(new MyProxy("1234"));
+    test(new MyProxy(undefined));
+    test(new MyProxy(new Object()));
+    trace("--- string null")
+    var s: String = null;
+    test(new MyProxy(s));
+    // coerce_to_primitive_side_effects_with_nulls is the same test, with these uncommented
+    // trace("--- normal null")
+    // test(new MyProxy(null));
+}
+main();

tests/tests/swfs/avm2/coerce_to_primitive_side_effects/output.txt

@@ -0,0 +1,29 @@
+----------- 1234
+call toString
+String
+1234
+call valueOf
+int
+1234
+----------- undefined
+call toString
+String
+undefined
+call valueOf
+int
+0
+----------- [object Object]
+call toString
+call valueOf
+Error 1050
+call valueOf
+call toString
+Error 1050
+--- string null
+----------- null
+call toString
+String
+null
+call valueOf
+int
+0

tests/tests/swfs/avm2/coerce_to_primitive_side_effects/test.toml

@@ -0,0 +1 @@
+num_frames = 1

tests/tests/swfs/avm2/coerce_to_primitive_side_effects_with_nulls/Test.as

@@ -0,0 +1,57 @@
+// compiled with mxmlc
+
+import flash.utils.getQualifiedClassName;
+import flash.utils.Proxy;
+import flash.utils.flash_proxy;
+
+package {
+    import flash.display.MovieClip;
+    public class Test extends MovieClip {
+    }
+}
+
+class MyProxy extends Proxy {
+    public var returnValue;
+    public function MyProxy(val) { this.returnValue = val; }
+    flash_proxy override function callProperty(name:*, ...rest):* {
+        trace("call " + name);
+        return this.returnValue;
+    }
+    flash_proxy override function getProperty(name:*):* {
+        trace("get " + name);
+        return undefined;
+    }
+    flash_proxy override function hasProperty(name:*):Boolean {
+        trace("has " + name);
+        return false;
+    }
+}
+
+function test(obj){
+    trace("----------- " + obj.returnValue);
+    var s: String;
+    var i: int;
+    try {
+        s = obj;
+        trace(getQualifiedClassName(s));
+        trace(s);
+    } catch (e) { trace("Error " + e.errorID); }
+
+    try {
+        i = obj;
+        trace(getQualifiedClassName(i));
+        trace(i);
+    } catch (e) { trace("Error " + e.errorID); }
+}
+
+function main(){
+    test(new MyProxy("1234"));
+    test(new MyProxy(undefined));
+    test(new MyProxy(new Object()));
+    trace("--- string null")
+    var s: String = null;
+    test(new MyProxy(s));
+    trace("--- normal null")
+    test(new MyProxy(null));
+}
+main();

tests/tests/swfs/avm2/coerce_to_primitive_side_effects_with_nulls/output.txt

@@ -0,0 +1,37 @@
+----------- 1234
+call toString
+String
+1234
+call valueOf
+int
+1234
+----------- undefined
+call toString
+String
+undefined
+call valueOf
+int
+0
+----------- [object Object]
+call toString
+call valueOf
+Error 1050
+call valueOf
+call toString
+Error 1050
+--- string null
+----------- null
+call toString
+String
+null
+call valueOf
+int
+0
+--- normal null
+----------- null
+call toString
+call valueOf
+Error 1050
+call valueOf
+call toString
+Error 1050

tests/tests/swfs/avm2/coerce_to_primitive_side_effects_with_nulls/test.toml

@@ -0,0 +1,4 @@
+num_frames = 1
+known_failure = true
+# this observes the subtle difference between string-null and object-null
+# the toString logic in AVM2 uses an "is Object" check via discriminant check, which only object-null passes.

tests/tests/swfs/from_avmplus/ecma3/Exceptions/exception_006_rt/test.toml

@@ -1,2 +1 @@
 num_ticks = 1
-known_failure = true