Skip to content

Commit

Permalink
Compiler: Change a new error from 1.9 to a warning
Browse files Browse the repository at this point in the history
To keep compatibility with existing Slint code

Commit 53e7900 added a call to
LayoutConstraints::new which is shown to produce error in the crater
run.
  • Loading branch information
ogoffart committed Dec 6, 2024
1 parent a7f0fdc commit 2ea482d
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 16 deletions.
18 changes: 11 additions & 7 deletions internal/compiler/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

//! Datastructures used to represent layouts in the compiler
use crate::diagnostics::BuildDiagnostics;
use crate::diagnostics::{BuildDiagnostics, DiagnosticLevel, Spanned};
use crate::expression_tree::*;
use crate::langtype::{ElementType, PropertyLookupResult, Type};
use crate::object_tree::{Component, ElementRc};
Expand Down Expand Up @@ -134,7 +134,11 @@ pub struct LayoutConstraints {
}

impl LayoutConstraints {
pub fn new(element: &ElementRc, diag: &mut BuildDiagnostics) -> Self {
/// Build the constraints for the given element
///
/// Report diagnostics when both constraints and fixed size are set
/// (one can set the level to warning to keep compatibility to old version of Slint)
pub fn new(element: &ElementRc, diag: &mut BuildDiagnostics, level: DiagnosticLevel) -> Self {
let mut constraints = Self {
min_width: binding_reference(element, "min-width"),
max_width: binding_reference(element, "max-width"),
Expand Down Expand Up @@ -162,14 +166,14 @@ impl LayoutConstraints {
&& old.priority.saturating_add(d2)
<= binding.priority.saturating_add(depth)
{
diag.push_error(
diag.push_diagnostic_with_span(
format!(
"Cannot specify both '{}' and '{}'",
prop,
"Cannot specify both '{prop}' and '{}'",
other_prop.name()
),
binding,
)
binding.to_source_location(),
level,
);
}
},
);
Expand Down
7 changes: 4 additions & 3 deletions internal/compiler/passes/default_geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use std::cell::RefCell;
use std::rc::Rc;

use crate::diagnostics::{BuildDiagnostics, Spanned};
use crate::diagnostics::{BuildDiagnostics, DiagnosticLevel, Spanned};
use crate::expression_tree::{
BindingExpression, BuiltinFunction, Expression, MinMaxOp, NamedReference, Unit,
};
Expand Down Expand Up @@ -187,7 +187,8 @@ fn gen_layout_info_prop(elem: &ElementRc, diag: &mut BuildDiagnostics) {
// FIXME: we should ideally add runtime code to merge layout info of all elements that are repeated (same as #407)
return None;
}
let explicit_constraints = LayoutConstraints::new(c, diag);
let explicit_constraints =
LayoutConstraints::new(c, diag, DiagnosticLevel::Error);
let use_implicit_size = c.borrow().builtin_type().map_or(false, |b| {
b.default_size_binding == DefaultSizeBinding::ImplicitSize
});
Expand Down Expand Up @@ -223,7 +224,7 @@ fn gen_layout_info_prop(elem: &ElementRc, diag: &mut BuildDiagnostics) {
let mut expr_h = implicit_layout_info_call(elem, Orientation::Horizontal);
let mut expr_v = implicit_layout_info_call(elem, Orientation::Vertical);

let explicit_constraints = LayoutConstraints::new(elem, diag);
let explicit_constraints = LayoutConstraints::new(elem, diag, DiagnosticLevel::Warning);
if !explicit_constraints.fixed_width {
merge_explicit_constraints(&mut expr_h, &explicit_constraints, Orientation::Horizontal);
}
Expand Down
9 changes: 4 additions & 5 deletions internal/compiler/passes/lower_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
use lyon_path::geom::euclid::approxeq::ApproxEq;

use crate::diagnostics::BuildDiagnostics;
use crate::diagnostics::Spanned;
use crate::diagnostics::{BuildDiagnostics, DiagnosticLevel, Spanned};
use crate::expression_tree::*;
use crate::langtype::ElementType;
use crate::langtype::Type;
Expand Down Expand Up @@ -43,7 +42,7 @@ pub fn lower_layouts(
});

*component.root_constraints.borrow_mut() =
LayoutConstraints::new(&component.root_element, diag);
LayoutConstraints::new(&component.root_element, diag, DiagnosticLevel::Error);

recurse_elem_including_sub_components(component, &(), &mut |elem, _| {
let component = elem.borrow().enclosing_component.upgrade().unwrap();
Expand Down Expand Up @@ -724,7 +723,7 @@ fn create_layout_item(
fix_explicit_percent("height", &rep_comp.root_element);

*rep_comp.root_constraints.borrow_mut() =
LayoutConstraints::new(&rep_comp.root_element, diag);
LayoutConstraints::new(&rep_comp.root_element, diag, DiagnosticLevel::Error);
rep_comp.root_element.borrow_mut().child_of_layout = true;
(
Some(if r.is_conditional_element {
Expand All @@ -738,7 +737,7 @@ fn create_layout_item(
(None, item_element.clone())
};

let constraints = LayoutConstraints::new(&actual_elem, diag);
let constraints = LayoutConstraints::new(&actual_elem, diag, DiagnosticLevel::Error);
Some(CreateLayoutItemResult {
item: LayoutItem { element: item_element.clone(), constraints },
elem: actual_elem,
Expand Down
25 changes: 24 additions & 1 deletion internal/compiler/tests/syntax/layout/min_max_conflict.slint
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright © SixtyFPS GmbH <[email protected]>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0

export Test := Rectangle {
export component Test inherits Rectangle {

GridLayout {
Rectangle {
Expand All @@ -18,5 +18,28 @@ export Test := Rectangle {
}
}

// outside a layout
Rectangle {
width: 42px;
// ^error{Cannot specify both 'width' and 'min-width'}
min-width: 5rem;
}

Rectangle {
// Slint 1.8 and earlier did not complain about extra specifications when the item is not in a layout
// contains a layout that's why it's a warning instead now
height: 10rem;
// ^warning{Cannot specify both 'height' and 'min-height'}
min-height: 8rem;
HorizontalLayout {
Rectangle {
height: 42px;
// ^error{Cannot specify both 'height' and 'max-height'}
max-height: 12px;

}
}
}


}

0 comments on commit 2ea482d

Please sign in to comment.