Compiler optimizations

internal/compiler/expression_tree.rs

@@ -1042,11 +1042,14 @@ impl Expression {
             Expression::ElementReference(_) => false,
             Expression::RepeaterIndexReference { .. } => false,
             Expression::RepeaterModelReference { .. } => false,
-            Expression::FunctionParameterReference { .. } => false,
+            // Allow functions to be marked as const
+            Expression::FunctionParameterReference { .. } => true,
             Expression::StructFieldAccess { base, .. } => base.is_constant(),
             Expression::ArrayIndex { array, index } => array.is_constant() && index.is_constant(),
             Expression::Cast { from, .. } => from.is_constant(),
-            Expression::CodeBlock(sub) => sub.len() == 1 && sub.first().unwrap().is_constant(),
+            // This is conservative: the return value is the last expression in the block, but
+            // we kind of mean "pure" here too, so ensure the whole body is OK.
+            Expression::CodeBlock(sub) => sub.iter().all(|s| s.is_constant()),
             Expression::FunctionCall { function, arguments, .. } => {
                 let is_const = match function {
                     Callable::Builtin(b) => b.is_const(),
@@ -1074,9 +1077,9 @@ impl Expression {
                 Path::Events(_, _) => true,
                 Path::Commands(_) => false,
             },
-            Expression::StoreLocalVariable { .. } => false,
-            // we should somehow find out if this is constant or not
-            Expression::ReadLocalVariable { .. } => false,
+            Expression::StoreLocalVariable { value, .. } => value.is_constant(),
+            // We only load what we store, and stores are alredy checked
+            Expression::ReadLocalVariable { .. } => true,
             Expression::EasingCurve(_) => true,
             Expression::LinearGradient { angle, stops } => {
                 angle.is_constant() && stops.iter().all(|(c, s)| c.is_constant() && s.is_constant())

internal/compiler/llr/pretty_print.rs

@@ -211,6 +211,9 @@ impl<T> Display for DisplayPropertyRef<'_, T> {
             }
             PropertyReference::InParent { level, parent_reference } => {
                 for _ in 0..level.get() {
+                    if ctx.parent.is_none() {
+                        return write!(f, "<invalid parent reference>");
+                    }
                     ctx = ctx.parent.unwrap().ctx;
                 }
                 write!(f, "{}", Self(parent_reference, ctx))

internal/compiler/passes.rs

@@ -198,7 +198,6 @@ pub async fn run_passes(
     unique_id::assign_unique_id(doc);
 
     doc.visit_all_used_components(|component| {
-        deduplicate_property_read::deduplicate_property_read(component);
         // Don't perform the empty rectangle removal when debug info is requested, because the resulting
         // item tree ends up with a hierarchy where certain items have children that aren't child elements
         // but siblings or sibling children. We need a new data structure to perform a correct element tree
@@ -217,6 +216,7 @@ pub async fn run_passes(
             // binding loop causes panics in const_propagation
             const_propagation::const_propagation(component);
         }
+        deduplicate_property_read::deduplicate_property_read(component);
         if !component.is_global() {
             resolve_native_classes::resolve_native_classes(component);
         }

internal/compiler/passes/const_propagation.rs

@@ -98,6 +98,29 @@ fn simplify_expression(expr: &mut Expression) -> bool {
                 }
                 ('|', Expression::BoolLiteral(false), e) => Some(std::mem::take(e)),
                 ('|', e, Expression::BoolLiteral(false)) => Some(std::mem::take(e)),
+                ('>', Expression::NumberLiteral(a, un1), Expression::NumberLiteral(b, un2))
+                    if un1 == un2 =>
+                {
+                    Some(Expression::BoolLiteral(*a > *b))
+                }
+                ('<', Expression::NumberLiteral(a, un1), Expression::NumberLiteral(b, un2))
+                    if un1 == un2 =>
+                {
+                    Some(Expression::BoolLiteral(*a < *b))
+                }
+                _ => None,
+            };
+            if let Some(new) = new {
+                *expr = new;
+            }
+            can_inline
+        }
+        Expression::UnaryOp { sub, op } => {
+            let can_inline = simplify_expression(sub);
+            let new = match (*op, &mut **sub) {
+                ('!', Expression::BoolLiteral(b)) => Some(Expression::BoolLiteral(!*b)),
+                ('-', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(-*n, *u)),
+                ('+', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(*n, *u)),
                 _ => None,
             };
             if let Some(new) = new {
@@ -148,6 +171,38 @@ fn simplify_expression(expr: &mut Expression) -> bool {
             }
             can_inline
         }
+        Expression::Condition { condition, true_expr, false_expr } => {
+            let mut can_inline = simplify_expression(condition);
+            can_inline &= match &**condition {
+                Expression::BoolLiteral(true) => {
+                    *expr = *true_expr.clone();
+                    simplify_expression(expr)
+                }
+                Expression::BoolLiteral(false) => {
+                    *expr = *false_expr.clone();
+                    simplify_expression(expr)
+                }
+                _ => simplify_expression(true_expr) && simplify_expression(false_expr),
+            };
+            can_inline
+        }
+        Expression::CodeBlock(stmts) if stmts.len() == 1 => {
+            *expr = stmts[0].clone();
+            simplify_expression(expr)
+        }
+        Expression::FunctionCall { function, arguments, .. } => {
+            let mut args_can_inline = true;
+            for arg in arguments.iter_mut() {
+                args_can_inline &= simplify_expression(arg);
+            }
+            if args_can_inline {
+                if let Some(inlined) = try_inline_function(function, arguments) {
+                    *expr = inlined;
+                    return true;
+                }
+            }
+            false
+        }
         Expression::ElementReference { .. } => false,
         Expression::LayoutCacheAccess { .. } => false,
         Expression::SolveLayout { .. } => false,
@@ -199,6 +254,30 @@ fn extract_constant_property_reference(nr: &NamedReference) -> Option<Expression
     Some(expression)
 }
 
+fn try_inline_function(function: &Callable, arguments: &[Expression]) -> Option<Expression> {
+    let Callable::Function(function) = function else {
+        return None;
+    };
+    if !function.is_constant() {
+        return None;
+    }
+    let mut body = extract_constant_property_reference(function)?;
+
+    body.visit_recursive_mut(&mut |e| {
+        if let Expression::FunctionParameterReference { index, ty } = e {
+            let Some(e_new) = arguments.get(*index).cloned() else { return };
+            if e_new.ty() == *ty {
+                *e = e_new;
+            }
+        }
+    });
+    if simplify_expression(&mut body) {
+        Some(body)
+    } else {
+        None
+    }
+}
+
 #[test]
 fn test() {
     let mut compiler_config =
@@ -210,12 +289,16 @@ fn test() {
 /* ... */
 struct Hello { s: string, v: float }
 global G {
-    property <float> p : 3 * 2 + 15 ;
+    pure function complicated(a: float ) -> bool { if a > 5 { return true; }; if a < 1 { return true; }; uncomplicated() }
+    pure function uncomplicated( ) -> bool { false }
+    out property <float> p : 3 * 2 + 15 ;
     property <string> q: "foo " + 42;
-    out property <Hello> out: { s: q, v: p };
+    out property <float> w : -p / 2;
+    out property <Hello> out: { s: q, v: complicated(w + 15) ? -123 : p };
 }
 export component Foo {
-    out property<float> out: G.out.v;
+    out property<float> out1: G.w;
+    out property<float> out2: G.out.v;
 }
 "#
         .into(),
@@ -226,20 +309,17 @@ export component Foo {
         spin_on::spin_on(crate::compile_syntax_node(doc_node, test_diags, compiler_config));
     assert!(!diag.has_errors(), "slint compile error {:#?}", diag.to_string_vec());
 
-    let out_binding = doc
-        .inner_components
-        .last()
-        .unwrap()
-        .root_element
-        .borrow()
-        .bindings
-        .get("out")
-        .unwrap()
-        .borrow()
-        .expression
-        .clone();
-    match &out_binding {
-        Expression::NumberLiteral(n, _) => assert_eq!(*n, (3 * 2 + 15) as f64),
-        _ => panic!("not number {out_binding:?}"),
+    let expected_p = 3.0 * 2.0 + 15.0;
+    let expected_w = -expected_p / 2.0;
+    let bindings = &doc.inner_components.last().unwrap().root_element.borrow().bindings;
+    let out1_binding = bindings.get("out1").unwrap().borrow().expression.clone();
+    match &out1_binding {
+        Expression::NumberLiteral(n, _) => assert_eq!(*n, expected_w),
+        _ => panic!("not number {out1_binding:?}"),
+    }
+    let out2_binding = bindings.get("out2").unwrap().borrow().expression.clone();
+    match &out2_binding {
+        Expression::NumberLiteral(n, _) => assert_eq!(*n, expected_p),
+        _ => panic!("not number {out2_binding:?}"),
     }
 }

internal/compiler/passes/inlining.rs

@@ -23,14 +23,15 @@ pub fn inline(doc: &Document, inline_selection: InlineSelection, diag: &mut Buil
     fn inline_components_recursively(
         component: &Rc<Component>,
         roots: &HashSet<ByAddress<Rc<Component>>>,
+        used_once: &HashSet<ByAddress<Rc<Component>>>,
         inline_selection: InlineSelection,
         diag: &mut BuildDiagnostics,
     ) {
         recurse_elem_no_borrow(&component.root_element, &(), &mut |elem, _| {
             let base = elem.borrow().base_type.clone();
             if let ElementType::Component(c) = base {
                 // First, make sure that the component itself is properly inlined
-                inline_components_recursively(&c, roots, inline_selection, diag);
+                inline_components_recursively(&c, roots, used_once, inline_selection, diag);
 
                 if c.parent_element.upgrade().is_some() {
                     // We should not inline a repeated element
@@ -43,28 +44,32 @@ pub fn inline(doc: &Document, inline_selection: InlineSelection, diag: &mut Buil
                     InlineSelection::InlineOnlyRequiredComponents => {
                         component_requires_inlining(&c)
                             || element_require_inlining(elem)
-                            // We always inline the root in case the element that instantiate this component needs full inlining
-                            || Rc::ptr_eq(elem, &component.root_element)
+                            // We always inline the root in case the element that instantiate this component needs full inlining,
+                            // except when the root is a repeater component, which are never inlined.
+                            || component.parent_element.upgrade().is_none() && Rc::ptr_eq(elem, &component.root_element)
                             // We always inline other roots as a component can't be both a sub component and a root
                             || roots.contains(&ByAddress(c.clone()))
+                            || used_once.contains(&ByAddress(c.clone()))
                     }
                 } {
                     inline_element(elem, &c, component, diag);
                 }
             }
         });
         component.popup_windows.borrow().iter().for_each(|p| {
-            inline_components_recursively(&p.component, roots, inline_selection, diag)
+            inline_components_recursively(&p.component, roots, used_once, inline_selection, diag)
         })
     }
     let mut roots = HashSet::new();
+    let mut used_once = HashSet::new();
     if inline_selection == InlineSelection::InlineOnlyRequiredComponents {
         for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
             roots.insert(ByAddress(component.clone()));
         }
+        used_once = collect_subcomponents_used_once(doc);
     }
     for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
-        inline_components_recursively(&component, &roots, inline_selection, diag);
+        inline_components_recursively(&component, &roots, &used_once, inline_selection, diag);
         let mut init_code = component.init_code.borrow_mut();
         let inlined_init_code = core::mem::take(&mut init_code.inlined_init_code);
         init_code.constructor_code.splice(0..0, inlined_init_code.into_values());
@@ -629,6 +634,12 @@ fn element_require_inlining(elem: &ElementRc) -> bool {
         // the generators assume that the children list is complete, which sub-components may break
         return true;
     }
+
+    // Popup windows need to be inlined for root.close() to work properly.
+    if super::lower_popups::is_popup_window(elem) {
+        return true;
+    }
+
     for (prop, binding) in &elem.borrow().bindings {
         if prop == "clip" {
             // otherwise the children of the clipped items won't get moved as child of the Clip element
@@ -644,3 +655,35 @@ fn element_require_inlining(elem: &ElementRc) -> bool {
 
     false
 }
+
+fn collect_subcomponents_used_once(doc: &Document) -> HashSet<ByAddress<Rc<Component>>> {
+    fn recursive(component: &Rc<Component>, hash: &mut HashMap<ByAddress<Rc<Component>>, bool>) {
+        match hash.entry(ByAddress(component.clone())) {
+            std::collections::hash_map::Entry::Occupied(mut o) => {
+                if !*o.get() {
+                    // We have already set this element (and its children to "set multiple times"
+                    return;
+                }
+                // Element used multiple times, visit all children to set them to multiple times too.
+                *o.get_mut() = false;
+            }
+            std::collections::hash_map::Entry::Vacant(v) => {
+                v.insert(true);
+            }
+        }
+
+        recurse_elem(&component.root_element, &(), &mut |elem: &ElementRc, &()| {
+            let ElementType::Component(base_comp) = &elem.borrow().base_type else { return };
+            recursive(base_comp, hash);
+        });
+        for popup in component.popup_windows.borrow().iter() {
+            recursive(&popup.component, hash);
+        }
+    }
+    // Stores true for elements that are used once, false for elements that are used multiple times;
+    let mut result = HashMap::new();
+    for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
+        recursive(&component, &mut result);
+    }
+    result.into_iter().filter(|(_, v)| *v).map(|(k, _)| k).collect()
+}

internal/compiler/passes/lower_popups.rs

@@ -25,20 +25,22 @@ pub fn lower_popups(
         component,
         &None,
         &mut |elem, parent_element: &Option<ElementRc>| {
-            let is_popup = match &elem.borrow().base_type {
-                ElementType::Builtin(base_type) => base_type.name == "PopupWindow",
-                ElementType::Component(base_type) => base_type.inherits_popup_window.get(),
-                _ => false,
-            };
-
-            if is_popup {
+            if is_popup_window(elem) {
                 lower_popup_window(elem, parent_element.as_ref(), &window_type, diag);
             }
             Some(elem.clone())
         },
     )
 }
 
+pub fn is_popup_window(element: &ElementRc) -> bool {
+    match &element.borrow().base_type {
+        ElementType::Builtin(base_type) => base_type.name == "PopupWindow",
+        ElementType::Component(base_type) => base_type.inherits_popup_window.get(),
+        _ => false,
+    }
+}
+
 fn lower_popup_window(
     popup_window_element: &ElementRc,
     parent_element: Option<&ElementRc>,

internal/compiler/passes/materialize_fake_properties.rs

@@ -149,15 +149,39 @@ pub fn initialize(elem: &ElementRc, name: &str) -> Option<Expression> {
         };
     }
 
+    // Hardcode properties for images, because this is a very common call, and this allows
+    // later optimization steps to eliminate these properties.
+    // Note that Rectangles and Empties are similarly optimized in layout_constraint_prop, and
+    // we rely on struct field access simplification for those.
+    if elem.borrow().builtin_type().map_or(false, |n| n.name == "Image") {
+        if elem.borrow().layout_info_prop(Orientation::Horizontal).is_none() {
+            match name {
+                "min-width" => return Some(Expression::NumberLiteral(0., Unit::Px)),
+                "max-width" => return Some(Expression::NumberLiteral(f32::MAX as _, Unit::Px)),
+                "horizontal-stretch" => return Some(Expression::NumberLiteral(0., Unit::None)),
+                _ => {}
+            }
+        }
+
+        if elem.borrow().layout_info_prop(Orientation::Vertical).is_none() {
+            match name {
+                "min-height" => return Some(Expression::NumberLiteral(0., Unit::Px)),
+                "max-height" => return Some(Expression::NumberLiteral(f32::MAX as _, Unit::Px)),
+                "vertical-stretch" => return Some(Expression::NumberLiteral(0., Unit::None)),
+                _ => {}
+            }
+        }
+    }
+
     let expr = match name {
         "min-height" => layout_constraint_prop(elem, "min", Orientation::Vertical),
         "min-width" => layout_constraint_prop(elem, "min", Orientation::Horizontal),
         "max-height" => layout_constraint_prop(elem, "max", Orientation::Vertical),
         "max-width" => layout_constraint_prop(elem, "max", Orientation::Horizontal),
-        "preferred-height" => layout_constraint_prop(elem, "preferred", Orientation::Vertical),
-        "preferred-width" => layout_constraint_prop(elem, "preferred", Orientation::Horizontal),
         "horizontal-stretch" => layout_constraint_prop(elem, "stretch", Orientation::Horizontal),
         "vertical-stretch" => layout_constraint_prop(elem, "stretch", Orientation::Vertical),
+        "preferred-height" => layout_constraint_prop(elem, "preferred", Orientation::Vertical),
+        "preferred-width" => layout_constraint_prop(elem, "preferred", Orientation::Horizontal),
         "opacity" => Expression::NumberLiteral(1., Unit::None),
         "visible" => Expression::BoolLiteral(true),
         _ => return None,