Add implicit coercions to remove most instances of manually constructing a `ReturnValue`.

core/src/avm1.rs

@@ -1,7 +1,6 @@
 use crate::avm1::function::Avm1Function;
 use crate::avm1::globals::create_globals;
 use crate::avm1::object::Object;
-use crate::avm1::return_value::ReturnValue;
 use crate::backend::navigator::NavigationMethod;
 use crate::context::UpdateContext;
 use crate::prelude::*;
@@ -110,7 +109,7 @@ impl<'gc> Avm1<'gc> {
             form_values.insert(
                 k,
                 v.ok()
-                    .unwrap_or(ReturnValue::Immediate(Value::Undefined))
+                    .unwrap_or_else(|| Value::Undefined.into())
                     .resolve(self, context)
                     .ok()
                     .unwrap_or(Value::Undefined)

core/src/avm1/activation.rs

@@ -268,13 +268,11 @@ impl<'gc> Activation<'gc> {
         context: &mut UpdateContext<'_, 'gc, '_>,
     ) -> Result<ReturnValue<'gc>, Error> {
         if name == "this" {
-            return Ok(ReturnValue::Immediate(Value::Object(self.this)));
+            return Ok(Value::Object(self.this).into());
         }
 
         if name == "arguments" && self.arguments.is_some() {
-            return Ok(ReturnValue::Immediate(Value::Object(
+            return Ok(Value::Object(self.arguments.unwrap()).into());
-                self.arguments.unwrap(),
-            )));
         }
 
         self.scope().resolve(name, avm, context, self.this)

core/src/avm1/function.rs

@@ -296,7 +296,7 @@ impl<'gc> Executable<'gc> {
                 }
                 avm.insert_stack_frame(frame_cell);
 
-                Ok(ReturnValue::ResultOf(frame_cell))
+                Ok(frame_cell.into())
             }
         }
     }

core/src/avm1/globals.rs

@@ -20,7 +20,7 @@ pub fn getURL<'a, 'gc>(
         let url = url_val.clone().into_string();
         if let Some(fscommand) = fscommand::parse(&url) {
             fscommand::handle(fscommand, avm, context);
-            return Ok(ReturnValue::Immediate(Value::Undefined));
+            return Ok(Value::Undefined.into());
         }
 
         let window = args.get(1).map(|v| v.clone().into_string());
@@ -34,7 +34,7 @@ pub fn getURL<'a, 'gc>(
         context.navigator.navigate_to_url(url, window, vars_method);
     }
 
-    Ok(ReturnValue::Immediate(Value::Undefined))
+    Ok(Value::Undefined.into())
 }
 
 pub fn random<'gc>(
@@ -44,10 +44,10 @@ pub fn random<'gc>(
     args: &[Value<'gc>],
 ) -> Result<ReturnValue<'gc>, Error> {
     match args.get(0) {
-        Some(Value::Number(max)) => Ok(ReturnValue::Immediate(Value::Number(
+        Some(Value::Number(max)) => {
-            action_context.rng.gen_range(0.0f64, max).floor(),
+            Ok(Value::Number(action_context.rng.gen_range(0.0f64, max).floor()).into())
-        ))),
+        }
-        _ => Ok(ReturnValue::Immediate(Value::Undefined)), //TODO: Shouldn't this be an error condition?
+        _ => Ok(Value::Undefined.into()), //TODO: Shouldn't this be an error condition?
     }
 }
 
@@ -58,11 +58,9 @@ pub fn boolean<'gc>(
     args: &[Value<'gc>],
 ) -> Result<ReturnValue<'gc>, Error> {
     if let Some(val) = args.get(0) {
-        Ok(ReturnValue::Immediate(Value::Bool(
+        Ok(Value::Bool(val.as_bool(avm.current_swf_version())).into())
-            val.as_bool(avm.current_swf_version()),
-        )))
     } else {
-        Ok(ReturnValue::Immediate(Value::Bool(false)))
+        Ok(Value::Bool(false).into())
     }
 }
 
@@ -73,9 +71,9 @@ pub fn number<'gc>(
     args: &[Value<'gc>],
 ) -> Result<ReturnValue<'gc>, Error> {
     if let Some(val) = args.get(0) {
-        Ok(ReturnValue::Immediate(Value::Number(val.as_number())))
+        Ok(Value::Number(val.as_number()).into())
     } else {
-        Ok(ReturnValue::Immediate(Value::Number(0.0)))
+        Ok(Value::Number(0.0).into())
     }
 }
 
@@ -86,11 +84,9 @@ pub fn is_nan<'gc>(
     args: &[Value<'gc>],
 ) -> Result<ReturnValue<'gc>, Error> {
     if let Some(val) = args.get(0) {
-        Ok(ReturnValue::Immediate(Value::Bool(
+        Ok(Value::Bool(val.as_number().is_nan()).into())
-            val.as_number().is_nan(),
-        )))
     } else {
-        Ok(ReturnValue::Immediate(Value::Bool(true)))
+        Ok(Value::Bool(true).into())
     }
 }
 

core/src/avm1/globals/math.rs

@@ -12,9 +12,9 @@ macro_rules! wrap_std {
                 $name,
                 |_avm, _context, _this, args| -> Result<ReturnValue<'gc>, Error> {
                     if let Some(input) = args.get(0) {
-                        Ok(ReturnValue::Immediate(Value::Number($std(input.as_number()))))
+                        Ok(Value::Number($std(input.as_number())).into())
                     } else {
-                        Ok(ReturnValue::Immediate(Value::Number(NAN)))
+                        Ok(Value::Number(NAN).into())
                     }
                 },
                 $gc_context,
@@ -32,16 +32,12 @@ fn atan2<'gc>(
 ) -> Result<ReturnValue<'gc>, Error> {
     if let Some(y) = args.get(0) {
         if let Some(x) = args.get(1) {
-            return Ok(ReturnValue::Immediate(Value::Number(
+            return Ok(Value::Number(y.as_number().atan2(x.as_number())).into());
-                y.as_number().atan2(x.as_number()),
-            )));
         } else {
-            return Ok(ReturnValue::Immediate(Value::Number(
+            return Ok(Value::Number(y.as_number().atan2(0.0)).into());
-                y.as_number().atan2(0.0),
-            )));
         }
     }
-    Ok(ReturnValue::Immediate(Value::Number(NAN)))
+    Ok(Value::Number(NAN).into())
 }
 
 pub fn random<'gc>(
@@ -50,9 +46,7 @@ pub fn random<'gc>(
     _this: GcCell<'gc, Object<'gc>>,
     _args: &[Value<'gc>],
 ) -> Result<ReturnValue<'gc>, Error> {
-    Ok(ReturnValue::Immediate(Value::Number(
+    Ok(Value::Number(action_context.rng.gen_range(0.0f64, 1.0f64)).into())
-        action_context.rng.gen_range(0.0f64, 1.0f64),
-    )))
 }
 
 pub fn create<'gc>(gc_context: MutationContext<'gc, '_>) -> GcCell<'gc, Object<'gc>> {
@@ -245,7 +239,7 @@ mod tests {
             let math = GcCell::allocate(context.gc_context, create(context.gc_context));
             assert_eq!(
                 atan2(avm, context, *math.read(), &[]).unwrap(),
-                ReturnValue::Immediate(Value::Number(NAN))
+                Value::Number(NAN).into()
             );
             assert_eq!(
                 atan2(
@@ -255,7 +249,7 @@ mod tests {
                     &[Value::Number(1.0), Value::Null]
                 )
                 .unwrap(),
-                ReturnValue::Immediate(Value::Number(NAN))
+                Value::Number(NAN).into()
             );
             assert_eq!(
                 atan2(
@@ -265,7 +259,7 @@ mod tests {
                     &[Value::Number(1.0), Value::Undefined]
                 )
                 .unwrap(),
-                ReturnValue::Immediate(Value::Number(NAN))
+                Value::Number(NAN).into()
             );
             assert_eq!(
                 atan2(
@@ -275,7 +269,7 @@ mod tests {
                     &[Value::Undefined, Value::Number(1.0)]
                 )
                 .unwrap(),
-                ReturnValue::Immediate(Value::Number(NAN))
+                Value::Number(NAN).into()
             );
         });
     }
@@ -286,7 +280,7 @@ mod tests {
             let math = GcCell::allocate(context.gc_context, create(context.gc_context));
             assert_eq!(
                 atan2(avm, context, *math.read(), &[Value::Number(10.0)]).unwrap(),
-                ReturnValue::Immediate(Value::Number(std::f64::consts::FRAC_PI_2))
+                Value::Number(std::f64::consts::FRAC_PI_2).into()
             );
             assert_eq!(
                 atan2(
@@ -296,7 +290,7 @@ mod tests {
                     &[Value::Number(1.0), Value::Number(2.0)]
                 )
                 .unwrap(),
-                ReturnValue::Immediate(Value::Number(f64::atan2(1.0, 2.0)))
+                Value::Number(f64::atan2(1.0, 2.0)).into()
             );
         });
     }

core/src/avm1/movie_clip.rs

@@ -14,10 +14,10 @@ macro_rules! with_movie_clip {
                 |_avm, _context, this, args| -> Result<ReturnValue<'gc>, Error> {
                     if let Some(display_object) = this.read().display_node() {
                         if let Some(movie_clip) = display_object.read().as_movie_clip() {
-                            return Ok(ReturnValue::Immediate($fn(movie_clip, args)));
+                            return Ok($fn(movie_clip, args).into());
                         }
                     }
-                    Ok(ReturnValue::Immediate(Value::Undefined))
+                    Ok(Value::Undefined.into())
                 },
                 $gc_context,
                 DontDelete | ReadOnly | DontEnum,
@@ -34,10 +34,10 @@ macro_rules! with_movie_clip_mut {
                 |_avm, context: &mut UpdateContext<'_, 'gc, '_>, this, args| -> Result<ReturnValue<'gc>, Error> {
                     if let Some(display_object) = this.read().display_node() {
                         if let Some(movie_clip) = display_object.write(context.gc_context).as_movie_clip_mut() {
-                            return Ok(ReturnValue::Immediate($fn(movie_clip, context, display_object, args)));
+                            return Ok($fn(movie_clip, context, display_object, args).into());
                         }
                     }
-                    Ok(ReturnValue::Immediate(Value::Undefined))
+                    Ok(Value::Undefined.into())
                 } as crate::avm1::function::NativeFunction<'gc>,
                 $gc_context,
                 DontDelete | ReadOnly | DontEnum,
@@ -59,7 +59,7 @@ pub fn overwrite_root<'gc>(
     this.write(ac.gc_context)
         .force_set("_root", new_val, EnumSet::new());
 
-    Ok(ReturnValue::Immediate(Value::Undefined))
+    Ok(Value::Undefined.into())
 }
 
 pub fn overwrite_global<'gc>(
@@ -75,7 +75,7 @@ pub fn overwrite_global<'gc>(
     this.write(ac.gc_context)
         .force_set("_global", new_val, EnumSet::new());
 
-    Ok(ReturnValue::Immediate(Value::Undefined))
+    Ok(Value::Undefined.into())
 }
 
 pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<'gc> {
@@ -120,18 +120,14 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<
 
     object.force_set_virtual(
         "_global",
-        Executable::Native(|avm, context, _this, _args| {
+        Executable::Native(|avm, context, _this, _args| Ok(avm.global_object(context).into())),
-            Ok(ReturnValue::Immediate(avm.global_object(context)))
-        }),
         Some(Executable::Native(overwrite_global)),
         EnumSet::new(),
     );
 
     object.force_set_virtual(
         "_root",
-        Executable::Native(|avm, context, _this, _args| {
+        Executable::Native(|avm, context, _this, _args| Ok(avm.root_object(context).into())),
-            Ok(ReturnValue::Immediate(avm.root_object(context)))
-        }),
         Some(Executable::Native(overwrite_root)),
         EnumSet::new(),
     );
@@ -139,14 +135,14 @@ pub fn create_movie_object<'gc>(gc_context: MutationContext<'gc, '_>) -> Object<
     object.force_set_virtual(
         "_parent",
         Executable::Native(|_avm, _context, this, _args| {
-            Ok(ReturnValue::Immediate(
+            Ok(this
-                this.read()
+                .read()
                 .display_node()
                 .and_then(|mc| mc.read().parent())
                 .and_then(|dn| dn.read().object().as_object().ok())
                 .map(|o| Value::Object(o.to_owned()))
-                    .unwrap_or(Value::Undefined),
+                .unwrap_or(Value::Undefined)
-            ))
+                .into())
         }),
         None,
         EnumSet::new(),

core/src/avm1/object.rs

@@ -57,7 +57,7 @@ impl<'gc> Property<'gc> {
     ) -> Result<ReturnValue<'gc>, Error> {
         match self {
             Property::Virtual { get, .. } => get.exec(avm, context, this, &[]),
-            Property::Stored { value, .. } => Ok(ReturnValue::Immediate(value.to_owned())),
+            Property::Stored { value, .. } => Ok(value.to_owned().into()),
         }
     }
 
@@ -318,7 +318,7 @@ impl<'gc> Object<'gc> {
             return value.get(avm, context, this);
         }
 
-        Ok(ReturnValue::Immediate(Value::Undefined))
+        Ok(Value::Undefined.into())
     }
 
     /// Delete a given value off the object.
@@ -364,7 +364,7 @@ impl<'gc> Object<'gc> {
         if let Some(function) = &self.function {
             function.exec(avm, context, this, args)
         } else {
-            Ok(ReturnValue::Immediate(Value::Undefined))
+            Ok(Value::Undefined.into())
         }
     }
 
@@ -458,7 +458,7 @@ mod tests {
                     .read()
                     .get("not_defined", avm, context, object)
                     .unwrap(),
-                ReturnValue::Immediate(Value::Undefined)
+                Value::Undefined.into()
             );
         })
     }
@@ -609,7 +609,7 @@ mod tests {
 
             assert_eq!(
                 object.read().get("virtual", avm, context, object).unwrap(),
-                ReturnValue::Immediate(Value::Undefined)
+                Value::Undefined.into()
             );
             assert_eq!(
                 object
@@ -620,7 +620,7 @@ mod tests {
             );
             assert_eq!(
                 object.read().get("stored", avm, context, object).unwrap(),
-                ReturnValue::Immediate(Value::Undefined)
+                Value::Undefined.into()
             );
             assert_eq!(
                 object
@@ -635,9 +635,7 @@ mod tests {
     #[test]
     fn test_iter_values() {
         with_object(0, |_avm, context, object| {
-            let getter = Executable::Native(|_avm, _context, _this, _args| {
+            let getter = Executable::Native(|_avm, _context, _this, _args| Ok(Value::Null.into()));
-                Ok(ReturnValue::Immediate(Value::Null))
-            });
 
             object
                 .write(context.gc_context)

core/src/avm1/return_value.rs

@@ -6,14 +6,28 @@ use crate::context::UpdateContext;
 use gc_arena::{Collect, GcCell};
 use std::fmt;
 
-/// Represents a value which can be returned immediately or at a later time.
+/// Represents the return value of a function call.
+///
+/// Since function calls can result in invoking native code or adding a new
+/// activation onto the AVM stack, you need another type to represent how the
+/// return value will be delivered to you.
+///
+/// This function contains a handful of utility methods for deciding what to do
+/// with a given value regardless of how it is delivered to the calling
+/// function.
+///
+/// It is `must_use` - failing to use a return value is a compiler warning. We
+/// provide a helper function specifically to indicate that you aren't
+/// interested in the result of a call.
 #[must_use = "Return values must be used"]
 #[derive(Clone)]
 pub enum ReturnValue<'gc> {
     /// Indicates that the return value is available immediately.
     Immediate(Value<'gc>),
 
-    /// Indicates that the return value will be calculated on the stack.
+    /// Indicates that the return value is the result of a given user-defined
+    /// function call. The activation record returned is the frame that needs
+    /// to return to get your value.
     ResultOf(GcCell<'gc, Activation<'gc>>),
 
     /// Indicates that there is no value to return.
@@ -125,3 +139,15 @@ impl<'gc> ReturnValue<'gc> {
         }
     }
 }
+
+impl<'gc> From<Value<'gc>> for ReturnValue<'gc> {
+    fn from(val: Value<'gc>) -> Self {
+        ReturnValue::Immediate(val)
+    }
+}
+
+impl<'gc> From<GcCell<'gc, Activation<'gc>>> for ReturnValue<'gc> {
+    fn from(frame: GcCell<'gc, Activation<'gc>>) -> Self {
+        ReturnValue::ResultOf(frame)
+    }
+}