Finish return statement handling

CHANGELOG.md

@@ -17,6 +17,8 @@ All notable changes to this project will be documented in this file.
  - color property to the Window element
  - maximum/minimum properties to the SpinBox
  - strings can contain escape code
+ - FocusScope to handle key events
+ - `return` statements
 
 
 ## [0.0.4] - 2020-12-04

api/sixtyfps-cpp/include/sixtyfps.h

@@ -209,6 +209,12 @@ vtable::Layout drop_in_place(ComponentRef component)
     reinterpret_cast<T *>(component.instance)->~T();
     return vtable::Layout { sizeof(T), alignof(T) };
 }
+
+template<typename T> struct ReturnWrapper {
+    ReturnWrapper(T val) : value(std::move(val)) {}
+    T value;
+};
+template<> struct ReturnWrapper<void> {};
 } // namespace private_api
 
 template<typename T>

sixtyfps_compiler/expression_tree.rs

@@ -471,7 +471,8 @@ impl Expression {
             Expression::ReadLocalVariable { ty, .. } => ty.clone(),
             Expression::EasingCurve(_) => Type::Easing,
             Expression::EnumerationValue(value) => Type::Enumeration(value.enumeration.clone()),
-            Expression::ReturnStatement(expr) => expr.as_ref().map_or(Type::Void, |expr| expr.ty()),
+            // invalid because the expression is unreachable
+            Expression::ReturnStatement(_) => Type::Invalid,
         }
     }
 
@@ -688,15 +689,12 @@ impl Expression {
         node: &impl SpannedWithSourceFile,
         diag: &mut BuildDiagnostics,
     ) -> Expression {
-        if let Expression::ReturnStatement(expr) = self {
-            return Expression::ReturnStatement(expr.map(|mut expr| {
-                let expr = std::mem::replace(&mut *expr, Expression::Invalid);
-                Box::new(expr.maybe_convert_to(target_type, node, diag))
-            }));
-        }
-
         let ty = self.ty();
-        if ty == target_type || target_type == Type::Void || target_type == Type::Invalid {
+        if ty == target_type
+            || target_type == Type::Void
+            || target_type == Type::Invalid
+            || ty == Type::Invalid
+        {
             self
         } else if ty.can_convert(&target_type) {
             let from = match (ty, &target_type) {
@@ -784,8 +782,6 @@ impl Expression {
                 _ => self,
             };
             Expression::Cast { from: Box::new(from), to: target_type }
-        } else if ty == Type::Invalid || target_type == Type::Invalid {
-            self
         } else if matches!((&ty, &target_type, &self), (Type::Array(a), Type::Array(b), Expression::Array{..})
             if a.can_convert(b) || **a == Type::Invalid)
         {

sixtyfps_compiler/generator/cpp.rs

@@ -356,7 +356,7 @@ fn handle_property_binding(
             callback_accessor_prefix = callback_accessor_prefix,
             prop = prop_name,
             params = params.join(", "),
-            code = compile_expression(binding_expression, &component)
+            code = compile_expression_wrap_return(binding_expression, &component)
         ));
     } else if let Expression::TwoWayBinding(nr, next) = &binding_expression {
         init.push(format!(
@@ -377,7 +377,7 @@ fn handle_property_binding(
 
         let component = &item.enclosing_component.upgrade().unwrap();
 
-        let init_expr = compile_expression(binding_expression, component);
+        let init_expr = compile_expression_wrap_return(binding_expression, component);
         let cpp_prop = if let Some(vp) = super::as_flickable_viewport_property(elem, prop_name) {
             format!("{}viewport.{}", accessor_prefix, vp,)
         } else {
@@ -1454,15 +1454,14 @@ fn compile_expression(
         Expression::CodeBlock(sub) => {
             let len = sub.len();
             let mut x = sub.iter().enumerate().map(|(i, e)| {
-                match e {
+                if i == len - 1 {
-                    Expression::ReturnStatement(return_expr) if i == len - 1 => {
+                    return_compile_expression(e, component, None) + ";"
-                        return_expr.as_ref().map_or_else(String::new, |return_expr| compile_expression(return_expr, component))
+                }
-                    },
+                else {
-                    e => compile_expression(e, component)
+                    compile_expression(e, component)
                 }
 
-            }).collect::<Vec<_>>();
+            });
-            if let Some(s) = x.last_mut() { *s = format!("return {};", s) };
 
             format!("[&]{{ {} }}()", x.join(";"))
         }
@@ -1552,27 +1551,19 @@ fn compile_expression(
             }
         }
         Expression::Condition { condition, true_expr, false_expr } => {
+            let ty = expr.ty();
             let cond_code = compile_expression(condition, component);
             let cond_code = remove_parentheses(&cond_code);
-            let true_code = compile_expression(true_expr, component);
+            let true_code = return_compile_expression(true_expr, component, Some(&ty));
-            let false_code = compile_expression(false_expr, component);
+            let false_code = return_compile_expression(false_expr, component, Some(&ty));
-            let ty = expr.ty();
-            if ty == Type::Invalid || ty == Type::Void {
             format!(
-                    r#"[&]() {{ if ({}) {{ {}; }} else {{ {}; }}}}()"#,
+                r#"[&]() -> {} {{ if ({}) {{ {}; }} else {{ {}; }}}}()"#,
+                ty.cpp_type().unwrap_or("void".into()),
                 cond_code,
                 true_code,
                 false_code
             )
-            } else {
+
-                format!(
-                    r#"[&]() -> {} {{ if ({}) {{ return {}; }} else {{ return {}; }}}}()"#,
-                    ty.cpp_type().unwrap(),
-                    cond_code,
-                    true_code,
-                    false_code
-                )
-            }
         }
         Expression::Array { element_ty, values } => {
             let ty = element_ty.cpp_type().unwrap_or_else(|| "FIXME: report error".to_owned());
@@ -1616,9 +1607,13 @@ fn compile_expression(
         Expression::EnumerationValue(value) => {
             format!("sixtyfps::{}::{}", value.enumeration.name, value.to_string())
         }
+        Expression::ReturnStatement(Some(expr)) => format!(
+            "throw sixtyfps::private_api::ReturnWrapper({})",
+            compile_expression(expr, component)
+        ),
+        Expression::ReturnStatement(None) => "throw sixtyfps::private_api::ReturnWrapper<void>()".to_owned(),
         Expression::Uncompiled(_) | Expression::TwoWayBinding(..) => panic!(),
         Expression::Invalid => "\n#error invalid expression\n".to_string(),
-        Expression::ReturnStatement(expr) => format!("return {};", expr.as_ref().map_or_else(String::new, |expr| compile_expression(expr, component))),
     }
 }
 
@@ -2290,3 +2285,62 @@ fn compile_path_events(events: &crate::expression_tree::PathEvents) -> (Vec<Stri
 
     (events, coordinates)
 }
+
+/// Like compile_expression, but wrap inside a try{}catch{} block to intercept the return
+fn compile_expression_wrap_return(expr: &Expression, component: &Rc<Component>) -> String {
+    /// Return a type if there is any `return` in sub expressions
+    fn return_type(expr: &Expression) -> Option<Type> {
+        if let Expression::ReturnStatement(val) = expr {
+            return Some(val.as_ref().map_or(Type::Void, |v| v.ty()));
+        }
+        let mut ret = None;
+        expr.visit(|e| {
+            if ret.is_none() {
+                ret = return_type(e)
+            }
+        });
+        ret
+    }
+
+    if let Some(ty) = return_type(expr) {
+        if ty == Type::Void || ty == Type::Invalid {
+            format!(
+                "[&]{{ try {{ {}; }} catch(const sixtyfps::private_api::ReturnWrapper<void> &w) {{ }} }}()",
+                compile_expression(expr, component)
+            )
+        } else {
+            let cpp_ty = ty.cpp_type().unwrap_or_default();
+            format!(
+                "[&]() -> {} {{ try {{ {}; }} catch(const sixtyfps::private_api::ReturnWrapper<{}> &w) {{ return w.value; }} }}()",
+                cpp_ty,
+                return_compile_expression(expr, component, Some(&ty)),
+                cpp_ty
+            )
+        }
+    } else {
+        compile_expression(expr, component)
+    }
+}
+
+/// Like compile expression, but prepended with `return` if not void.
+/// ret_type is the expecting type that should be returned with that return statement
+fn return_compile_expression(
+    expr: &Expression,
+    component: &Rc<Component>,
+    ret_type: Option<&Type>,
+) -> String {
+    let e = compile_expression(expr, component);
+    if ret_type == Some(&Type::Void) || ret_type == Some(&Type::Invalid) {
+        e
+    } else {
+        let ty = expr.ty();
+        if ty == Type::Invalid && ret_type.is_some() {
+            // e is unreachable so it probably throws. But we still need to return something to avoid a warning
+            format!("{}; return {{}}", e)
+        } else if ty == Type::Invalid || ty == Type::Void {
+            e
+        } else {
+            format!("return {}", e)
+        }
+    }
+}

sixtyfps_compiler/generator/rust.rs

@@ -175,7 +175,8 @@ fn handle_property_binding(
     } else {
         let tokens_for_expression = compile_expression(binding_expression, &component);
         init.push(if binding_expression.is_constant() {
-            quote! { #rust_property.set((#tokens_for_expression) as _); }
+            let t = rust_type(&prop_type, &Default::default()).unwrap_or(quote!(_));
+            quote! { #rust_property.set((||-> #t { (#tokens_for_expression) as #t })()); }
         } else {
             let binding_tokens = quote!({
                 let self_weak = sixtyfps::re_exports::VRc::downgrade(&self_pinned);

sixtyfps_runtime/interpreter/eval.rs

@@ -275,6 +275,8 @@ pub struct EvalLocalContext<'a, 'id> {
     local_variables: HashMap<String, Value>,
     function_arguments: Vec<Value>,
     component_instance: ComponentInstance<'a, 'id>,
+    /// When Some, a return statement was executed and one must stop evaluating
+    return_value: Option<Value>,
 }
 
 impl<'a, 'id> EvalLocalContext<'a, 'id> {
@@ -283,6 +285,7 @@ impl<'a, 'id> EvalLocalContext<'a, 'id> {
             local_variables: Default::default(),
             function_arguments: Default::default(),
             component_instance: ComponentInstance::InstanceRef(component),
+            return_value: None,
         }
     }
 
@@ -295,6 +298,7 @@ impl<'a, 'id> EvalLocalContext<'a, 'id> {
             component_instance: ComponentInstance::InstanceRef(component),
             function_arguments,
             local_variables: Default::default(),
+            return_value: None,
         }
     }
 
@@ -303,12 +307,16 @@ impl<'a, 'id> EvalLocalContext<'a, 'id> {
             local_variables: Default::default(),
             function_arguments: Default::default(),
             component_instance: ComponentInstance::GlobalComponent(&global),
+            return_value: None,
         }
     }
 }
 
 /// Evaluate an expression and return a Value as the result of this expression
 pub fn eval_expression(e: &Expression, local_context: &mut EvalLocalContext) -> Value {
+    if let Some(r) = &local_context.return_value {
+        return r.clone();
+    }
     match e {
         Expression::Invalid => panic!("invalid expression while evaluating"),
         Expression::Uncompiled(_) => panic!("uncompiled expression while evaluating"),
@@ -360,10 +368,10 @@ pub fn eval_expression(e: &Expression, local_context: &mut EvalLocalContext) ->
         Expression::CodeBlock(sub) => {
             let mut v = Value::Void;
             for e in sub {
-                if let Expression::ReturnStatement(expr) = e {
-                    return expr.as_ref().map_or(Value::Void, |expr| eval_expression(expr, local_context))
-                }
                 v = eval_expression(e, local_context);
+                if let Some(r) = &local_context.return_value {
+                    return r.clone();
+                }
             }
             v
         }
@@ -577,7 +585,7 @@ pub fn eval_expression(e: &Expression, local_context: &mut EvalLocalContext) ->
             {
                 Ok(true) => eval_expression(&**true_expr, local_context),
                 Ok(false) => eval_expression(&**false_expr, local_context),
-                _ => panic!("conditional expression did not evaluate to boolean"),
+                _ => local_context.return_value.clone().expect("conditional expression did not evaluate to boolean"),
             }
         }
         Expression::Array { values, .. } => Value::Array(
@@ -609,7 +617,13 @@ pub fn eval_expression(e: &Expression, local_context: &mut EvalLocalContext) ->
         Expression::EnumerationValue(value) => {
             Value::EnumerationValue(value.enumeration.name.clone(), value.to_string())
         }
-        Expression::ReturnStatement(_) => panic!("internal error: return statement must only appear inside code block and handled there")
+        Expression::ReturnStatement(x) => {
+            let val = x.as_ref().map_or(Value::Void, |x| eval_expression(&x, local_context));
+            if local_context.return_value.is_none() {
+                local_context.return_value = Some(val);
+            }
+            local_context.return_value.clone().unwrap()
+        }
     }
 }
 

tests/cases/expr/return.60

@@ -7,16 +7,20 @@
     This file is also available under commercial licensing terms.
     Please contact info@sixtyfps.io for more information.
 LICENSE END */
- TestCase := Rectangle {
+TestCase := Rectangle {
-    property <bool> toggle;
+    property <bool> toggle: { return false; }
     property <int> value: {
+        debug("XXX \{ toggle ? "true" : "false" }");
         if (toggle) {
+            debug("HI");
             return 42;
+            debug("HO");
         }
+        debug("NOP");
         return 100;
     }
 
-    signal test_signal;
+    callback test_signal;
     property <bool> block_signal;
     property <bool> signal_handled;
     test_signal => {
@@ -43,4 +47,35 @@ instance.emit_test_signal();
 assert(!instance.get_signal_handled());
 ```
 
+```rust
+let instance = TestCase::new();
+assert_eq!(instance.get_value(), 100);
+instance.set_toggle(true);
+assert_eq!(instance.get_value(), 42);
+
+instance.emit_test_signal();
+assert!(instance.get_signal_handled());
+
+instance.set_signal_handled(false);
+instance.set_block_signal(true);
+instance.emit_test_signal();
+assert!(!instance.get_signal_handled());
+```
+
+
+```js
+var instance = new sixtyfps.TestCase({});
+assert.equal(instance.value, 100);
+instance.toggle = (true);
+assert.equal(instance.value, 42);
+
+instance.test_signal();
+assert(instance.signal_handled);
+
+instance.signal_handled = (false);
+instance.block_signal = (true);
+instance.test_signal();
+assert(!instance._signal_handled);
+```
+
 */

tests/driver/cppdriver.rs

@@ -129,7 +129,7 @@ pub fn test(testcase: &test_driver_lib::TestCase) -> Result<(), Box<dyn Error>>
         if let Some(exit_code) = output.status.code() {
             return Err(format!("Test case exited with non-zero code: {}", exit_code).into());
         } else {
-            return Err("Test case exited by callback".into());
+            return Err("Test case exited by signal".into());
         }
     }