Skip to content

Commit

Permalink
Detect dingind loops that applies to the Window itself
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ogoffart committed Nov 23, 2023
1 parent b19cbba commit 631de0e
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 5 deletions.
55 changes: 50 additions & 5 deletions internal/compiler/passes/binding_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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}
Expand Down
13 changes: 13 additions & 0 deletions internal/compiler/tests/syntax/analysis/binding_loop_layout2.slint
Original file line number Diff line number Diff line change
Expand Up @@ -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 <image> 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;
}
Expand Down
25 changes: 25 additions & 0 deletions internal/compiler/tests/syntax/analysis/binding_loop_window.slint
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright © SixtyFPS GmbH <info@slint.dev>
// 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";
}
}
}
}
16 changes: 16 additions & 0 deletions internal/compiler/tests/syntax/analysis/binding_loop_window2.slint
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright © SixtyFPS GmbH <info@slint.dev>
// 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 {}
}
}

0 comments on commit 631de0e

Please sign in to comment.