Skip to content

Commit d030fad

Browse files
committed
compiler: forbid setting row/col in a callback, in a grid with auto numbering
1 parent c86bfbf commit d030fad

File tree

4 files changed

+55
-0
lines changed

4 files changed

+55
-0
lines changed

internal/compiler/layout.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,9 @@ pub struct GridLayout {
468468
/// When this GridLayout is actually the layout of a Dialog, then the cells start with all the buttons,
469469
/// and this variable contains their roles. The string is actually one of the values from the i_slint_core::layout::DialogButtonRole
470470
pub dialog_button_roles: Option<Vec<SmolStr>>,
471+
472+
/// Whether any of the row/column expressions use 'auto'
473+
pub uses_auto: bool,
471474
}
472475

473476
impl GridLayout {

internal/compiler/passes/binding_analysis.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,31 @@ fn analyze_element(
261261
process_property(&h.clone().into(), P, context, reverse_aliases, diag);
262262
process_property(&v.clone().into(), P, context, reverse_aliases, diag);
263263
}
264+
265+
for info in elem.borrow().debug.iter() {
266+
if let Some(crate::layout::Layout::GridLayout(grid)) = &info.layout {
267+
if grid.uses_auto {
268+
for rowcol_prop_name in ["row", "col"] {
269+
for it in grid.elems.iter() {
270+
let child = &it.item.element;
271+
if child
272+
.borrow()
273+
.property_analysis
274+
.borrow()
275+
.get(rowcol_prop_name)
276+
.is_some_and(|a| a.is_set || a.is_set_externally)
277+
{
278+
diag.push_error(
279+
format!("Cannot set property '{}' on '{}' because parent GridLayout uses auto-numbering",
280+
rowcol_prop_name, child.borrow().id),
281+
&child.borrow().to_source_location(), // not ideal, the location of the property being set would be better
282+
);
283+
}
284+
}
285+
}
286+
}
287+
}
288+
}
264289
}
265290

266291
#[derive(Copy, Clone, dm::BitAnd, dm::BitOr, dm::BitAndAssign, dm::BitOrAssign)]

internal/compiler/passes/lower_layout.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ fn lower_grid_layout(
181181
elems: Default::default(),
182182
geometry: LayoutGeometry::new(grid_layout_element),
183183
dialog_button_roles: None,
184+
uses_auto: false,
184185
};
185186

186187
let layout_organized_data_prop = create_new_prop(
@@ -264,6 +265,7 @@ fn lower_grid_layout(
264265
}
265266
}
266267
grid_layout_element.borrow_mut().children = collected_children;
268+
grid.uses_auto = numbering_type == Some(RowColExpressionType::Auto);
267269
let span = grid_layout_element.borrow().to_source_location();
268270

269271
layout_organized_data_prop.element().borrow_mut().bindings.insert(
@@ -630,6 +632,7 @@ fn lower_dialog_layout(
630632
elems: Default::default(),
631633
geometry: LayoutGeometry::new(dialog_element),
632634
dialog_button_roles: None,
635+
uses_auto: true,
633636
};
634637
let metrics = &style_metrics.root_element;
635638
grid.geometry
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright © Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com
2+
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
3+
4+
export component TestCase inherits Window {
5+
6+
lay1 := GridLayout {
7+
spacing: 20px;
8+
text1 := Text {
9+
// ^error{Cannot set property 'col' on 'text1' because parent GridLayout uses auto-numbering}
10+
row: 0;
11+
col: 1;
12+
text: "text 1";
13+
}
14+
text2 := Text {
15+
// ^error{Cannot set property 'row' on 'text2' because parent GridLayout uses auto-numbering}
16+
text: "text 2";
17+
}
18+
}
19+
20+
init => {
21+
text1.col = 3; // should be an error
22+
text2.row = 2; // too (making sure this is detected even when row is not set initially)
23+
}
24+
}

0 commit comments

Comments
 (0)