From 631de0e45544e934b3818b3baeea80e06d1d17e7 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 23 Nov 2023 17:31:18 +0100 Subject: [PATCH] Detect dingind loops that applies to the Window itself The Window geometry depends on its constraints, so its constraints cannot depends on its geometry This fixes Infinitely growing layout, and other panics Fixes #3989 Fixes #2902 --- internal/compiler/passes/binding_analysis.rs | 55 +++++++++++++++++-- .../syntax/analysis/binding_loop_layout.slint | 2 + .../analysis/binding_loop_layout2.slint | 13 +++++ .../syntax/analysis/binding_loop_window.slint | 25 +++++++++ .../analysis/binding_loop_window2.slint | 16 ++++++ 5 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 internal/compiler/tests/syntax/analysis/binding_loop_window.slint create mode 100644 internal/compiler/tests/syntax/analysis/binding_loop_window2.slint diff --git a/internal/compiler/passes/binding_analysis.rs b/internal/compiler/passes/binding_analysis.rs index 92a8b26cd3e..c3f3aff116a 100644 --- a/internal/compiler/passes/binding_analysis.rs +++ b/internal/compiler/passes/binding_analysis.rs @@ -344,13 +344,20 @@ fn process_property( if element.borrow().bindings.contains_key(prop.prop.name()) { analyze_binding(&prop, context, reverse_aliases, diag); } - let next = if let ElementType::Component(base) = &element.borrow().base_type { - if element.borrow().property_declarations.contains_key(prop.prop.name()) { + let next = match &element.borrow().base_type { + ElementType::Component(base) => { + if element.borrow().property_declarations.contains_key(prop.prop.name()) { + break; + } + base.root_element.clone() + } + ElementType::Builtin(builtin) => { + if builtin.properties.contains_key(prop.prop.name()) { + visit_builtin_property(builtin, &prop, context, reverse_aliases, diag); + } break; } - base.root_element.clone() - } else { - break; + _ => break, }; next.borrow() .property_analysis @@ -496,6 +503,44 @@ fn visit_implicit_layout_info_dependencies( } } +fn visit_builtin_property( + builtin: &crate::langtype::BuiltinElement, + prop: &PropertyPath, + context: &mut AnalysisContext, + reverse_aliases: &ReverseAliases, + diag: &mut BuildDiagnostics, +) { + let name = prop.prop.name(); + if builtin.name == "Window" { + for (p, orientation) in + [("width", Orientation::Horizontal), ("height", Orientation::Vertical)] + { + if name == p { + // find the actual root component + let is_root = |e: &ElementRc| -> bool { + ElementRc::ptr_eq( + e, + &e.borrow().enclosing_component.upgrade().unwrap().root_element, + ) + }; + let mut root = prop.prop.element(); + if !is_root(&root) { + return; + }; + for e in prop.elements.iter().rev() { + if !is_root(&e.0) { + return; + } + root = e.0.clone(); + } + if let Some(p) = root.borrow().layout_info_prop(orientation) { + process_property(&p.clone().into(), context, reverse_aliases, diag); + }; + } + } + } +} + /// Make sure that the is_set property analysis is set to any property which has a two way binding /// to a property that is, itself, is set /// diff --git a/internal/compiler/tests/syntax/analysis/binding_loop_layout.slint b/internal/compiler/tests/syntax/analysis/binding_loop_layout.slint index f697553a25c..2c1cb02bb81 100644 --- a/internal/compiler/tests/syntax/analysis/binding_loop_layout.slint +++ b/internal/compiler/tests/syntax/analysis/binding_loop_layout.slint @@ -20,6 +20,7 @@ TC := Rectangle { export Test := Rectangle { +// ^error{The binding for the property 'width' is part of a binding loop} VerticalLayout { // ^error{The binding for the property 'layoutinfo-h' is part of a binding loop} // FIXME: That's an internal property, but people might understand // ^^error{The binding for the property 'min-width' is part of a binding loop} @@ -37,6 +38,7 @@ export Test := Rectangle { // ^^^^error{The binding for the property 'preferred-height' is part of a binding loop} // ^^^^^error{The binding for the property 'width' is part of a binding loop} // ^^^^^^error{The binding for the property 'layout-cache' is part of a binding loop} +// ^^^^^^^error{The binding for the property 'width' is part of a binding loop} Text { text: "hello \{l.preferred-width/1px}x\{l.preferred-height/1px}"; // ^error{The binding for the property 'text' is part of a binding loop} diff --git a/internal/compiler/tests/syntax/analysis/binding_loop_layout2.slint b/internal/compiler/tests/syntax/analysis/binding_loop_layout2.slint index e02e5e0aee9..bf4865a71ec 100644 --- a/internal/compiler/tests/syntax/analysis/binding_loop_layout2.slint +++ b/internal/compiler/tests/syntax/analysis/binding_loop_layout2.slint @@ -27,12 +27,25 @@ Wrap := Rectangle { } export Test := Rectangle { +// ^error{The binding for the property 'width' is part of a binding loop} +// ^^error{The binding for the property 'layoutinfo-v' is part of a binding loop} +// ^^^error{The binding for the property 'layoutinfo-v' is part of a binding loop} +// ^^^^error{The binding for the property 'height' is part of a binding loop} +// ^^^^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} +// ^^^^^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + property source; GridLayout { // ^error{The binding for the property 'layout-cache-h' is part of a binding loop} // ^^error{The binding for the property 'width' is part of a binding loop} // ^^^error{The binding for the property 'layout-cache-v' is part of a binding loop} // ^^^^error{The binding for the property 'height' is part of a binding loop} +// ^^^^^error{The binding for the property 'width' is part of a binding loop} +// ^^^^^^error{The binding for the property 'layoutinfo-v' is part of a binding loop} +// ^^^^^^^error{The binding for the property 'height' is part of a binding loop} +// ^^^^^^^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + + Image { source: root.source; } diff --git a/internal/compiler/tests/syntax/analysis/binding_loop_window.slint b/internal/compiler/tests/syntax/analysis/binding_loop_window.slint new file mode 100644 index 00000000000..167982cc32d --- /dev/null +++ b/internal/compiler/tests/syntax/analysis/binding_loop_window.slint @@ -0,0 +1,25 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial + +// Issue #2902 + +import { HorizontalBox, VerticalBox } from "std-widgets.slint"; + +export component MainWindow inherits Window { +// ^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + + HorizontalBox { +// ^error{The binding for the property 'width' is part of a binding loop} +// ^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + + VerticalBox { + + width: parent.width; +// ^error{The binding for the property 'width' is part of a binding loop} + //height: parent.height; + Text { + text: "Test"; + } + } + } +} \ No newline at end of file diff --git a/internal/compiler/tests/syntax/analysis/binding_loop_window2.slint b/internal/compiler/tests/syntax/analysis/binding_loop_window2.slint new file mode 100644 index 00000000000..39a1005e066 --- /dev/null +++ b/internal/compiler/tests/syntax/analysis/binding_loop_window2.slint @@ -0,0 +1,16 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial + +// Issue #3898 +export component LspCrash inherits Rectangle { +// ^error{The binding for the property 'width' is part of a binding loop} +// ^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} +// ^^^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + + HorizontalLayout { +// ^error{The binding for the property 'layoutinfo-h' is part of a binding loop} + padding-left: parent.width * 0.015; +// ^error{The binding for the property 'padding-left' is part of a binding loop} + Rectangle {} + } +}