Skip to content

Commit 39671ab

Browse files
committed
LLR: don't use LocalMemberReference to initialize property binding
Because they could be aliases to global
1 parent 0399d0a commit 39671ab

File tree

7 files changed

+94
-37
lines changed

7 files changed

+94
-37
lines changed

internal/compiler/generator/cpp.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -597,13 +597,13 @@ fn property_set_value_code(
597597
}
598598

599599
fn handle_property_init(
600-
prop: &llr::LocalMemberReference,
600+
prop: &llr::MemberReference,
601601
binding_expression: &llr::BindingExpression,
602602
init: &mut Vec<String>,
603603
ctx: &EvaluationContext,
604604
) {
605-
let prop_access = access_local_member(prop, ctx);
606-
let prop_type = ctx.relative_property_ty(prop, 0);
605+
let prop_access = access_member(prop, ctx).unwrap();
606+
let prop_type = ctx.property_ty(prop);
607607
if let Type::Callback(callback) = &prop_type {
608608
let mut ctx2 = ctx.clone();
609609
ctx2.argument_types = &callback.args;
@@ -2086,8 +2086,8 @@ fn generate_sub_component(
20862086

20872087
for (prop1, prop2, fields) in &component.two_way_bindings {
20882088
if fields.is_empty() {
2089-
let ty = ctx.relative_property_ty(&prop1, 0).cpp_type().unwrap();
2090-
let p1 = access_local_member(prop1, &ctx);
2089+
let ty = ctx.property_ty(&prop1).cpp_type().unwrap();
2090+
let p1 = access_member(prop1, &ctx).unwrap();
20912091
init.push(
20922092
access_member(prop2, &ctx).then(|p2| {
20932093
format!("slint::private_api::Property<{ty}>::link_two_way(&{p1}, &{p2})",)
@@ -2105,7 +2105,7 @@ fn generate_sub_component(
21052105
ty = s.fields.get(f).unwrap();
21062106
}
21072107

2108-
let p1 = access_local_member(prop1, &ctx);
2108+
let p1 = access_member(prop1, &ctx).unwrap();
21092109
init.push(
21102110
access_member(prop2, &ctx).then(|p2|
21112111
format!("slint::private_api::Property<{cpp_ty}>::link_two_way_with_map(&{p2}, &{p1}, [](const auto &x){{ return {access}; }}, [](auto &x, const auto &v){{ {access} = v; }})")
@@ -2121,7 +2121,7 @@ fn generate_sub_component(
21212121
}
21222122
}
21232123
for prop in &component.const_properties {
2124-
if component.prop_used(prop, root) {
2124+
if component.prop_used(&prop.clone().into(), root) {
21252125
let p = access_local_member(prop, &ctx);
21262126
properties_init_code.push(format!("{p}.set_constant();"));
21272127
}
@@ -2659,10 +2659,7 @@ fn generate_global(
26592659

26602660
if let Some(expression) = expression.as_ref() {
26612661
handle_property_init(
2662-
&llr::LocalMemberReference {
2663-
sub_component_path: vec![],
2664-
reference: property_index.into(),
2665-
},
2662+
&llr::LocalMemberReference::from(property_index).into(),
26662663
expression,
26672664
&mut init,
26682665
&ctx,

internal/compiler/generator/rust.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -577,13 +577,13 @@ fn generate_enum(en: &std::rc::Rc<Enumeration>) -> TokenStream {
577577
}
578578

579579
fn handle_property_init(
580-
prop: &llr::LocalMemberReference,
580+
prop: &llr::MemberReference,
581581
binding_expression: &llr::BindingExpression,
582582
init: &mut Vec<TokenStream>,
583583
ctx: &EvaluationContext,
584584
) {
585-
let rust_property = access_local_member(prop, ctx);
586-
let prop_type = ctx.relative_property_ty(prop, 0);
585+
let rust_property = access_member(prop, ctx).unwrap();
586+
let prop_type = ctx.property_ty(prop);
587587

588588
let init_self_pin_ref = if ctx.current_global().is_some() {
589589
quote!(let _self = self_rc.as_ref();)
@@ -1125,7 +1125,7 @@ fn generate_sub_component(
11251125
component.popup_windows.iter().enumerate().map(|(i, _)| internal_popup_id(i));
11261126

11271127
for (prop1, prop2, fields) in &component.two_way_bindings {
1128-
let p1 = access_local_member(prop1, &ctx);
1128+
let p1 = access_member(prop1, &ctx).unwrap();
11291129
let p2 = access_member(prop2, &ctx);
11301130
let r = p2.then(|p2| {
11311131
if fields.is_empty() {
@@ -1151,7 +1151,7 @@ fn generate_sub_component(
11511151
}
11521152
}
11531153
for prop in &component.const_properties {
1154-
if component.prop_used(prop, root) {
1154+
if component.prop_used(&prop.clone().into(), root) {
11551155
let rust_property = access_local_member(prop, &ctx);
11561156
init.push(quote!(#rust_property.set_constant();))
11571157
}
@@ -1487,7 +1487,12 @@ fn generate_global(
14871487
continue;
14881488
}
14891489
if let Some(expression) = expression.as_ref() {
1490-
handle_property_init(&property_index.into(), expression, &mut init, &ctx)
1490+
handle_property_init(
1491+
&llr::LocalMemberReference::from(property_index).into(),
1492+
expression,
1493+
&mut init,
1494+
&ctx,
1495+
)
14911496
}
14921497
}
14931498
for (property_index, cst) in global.const_properties.iter_enumerated() {

internal/compiler/llr/expression.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ impl<'a, T> EvaluationContext<'a, T> {
567567
};
568568

569569
let animation = sc.animations.get(prop).map(|a| (a, map.clone()));
570-
let analysis = sc.prop_analysis.get(prop);
570+
let analysis = sc.prop_analysis.get(&prop.clone().into());
571571
if let Some(a) = &analysis {
572572
if let Some(init) = a.property_init {
573573
return PropertyInfoResult {

internal/compiler/llr/item_tree.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,12 @@ pub struct SubComponent {
327327
pub sub_components: TiVec<SubComponentInstanceIdx, SubComponentInstance>,
328328
/// The initial value or binding for properties.
329329
/// This is ordered in the order they must be set.
330-
pub property_init: Vec<(LocalMemberReference, BindingExpression)>,
331-
pub change_callbacks: Vec<(LocalMemberReference, MutExpression)>,
330+
pub property_init: Vec<(MemberReference, BindingExpression)>,
331+
pub change_callbacks: Vec<(MemberReference, MutExpression)>,
332332
/// The animation for properties which are animated
333333
pub animations: HashMap<LocalMemberReference, Expression>,
334334
/// The two way bindings that map the first property to the second wih optional field access
335-
pub two_way_bindings: Vec<(LocalMemberReference, MemberReference, Vec<SmolStr>)>,
335+
pub two_way_bindings: Vec<(MemberReference, MemberReference, Vec<SmolStr>)>,
336336
pub const_properties: Vec<LocalMemberReference>,
337337
/// Code that is run in the sub component constructor, after property initializations
338338
pub init_code: Vec<MutExpression>,
@@ -349,7 +349,7 @@ pub struct SubComponent {
349349
/// Maps item index to a list of encoded element infos of the element (type name, qualified ids).
350350
pub element_infos: BTreeMap<u32, String>,
351351

352-
pub prop_analysis: HashMap<LocalMemberReference, PropAnalysis>,
352+
pub prop_analysis: HashMap<MemberReference, PropAnalysis>,
353353
}
354354

355355
#[derive(Debug)]
@@ -401,10 +401,16 @@ impl SubComponent {
401401
}
402402

403403
/// Return if a local property is used. (unused property shouldn't be generated)
404-
pub fn prop_used(&self, prop: &LocalMemberReference, cu: &CompilationUnit) -> bool {
405-
if let LocalMemberIndex::Property(property_index) = &prop.reference {
404+
pub fn prop_used(&self, prop: &MemberReference, cu: &CompilationUnit) -> bool {
405+
let MemberReference::Relative { parent_level, local_reference } = prop else {
406+
return true;
407+
};
408+
if *parent_level != 0 {
409+
return true;
410+
}
411+
if let LocalMemberIndex::Property(property_index) = &local_reference.reference {
406412
let mut sc = self;
407-
for i in &prop.sub_component_path {
413+
for i in &local_reference.sub_component_path {
408414
sc = &cu.sub_components[sc.sub_components[*i].ty];
409415
}
410416
if sc.properties[*property_index].use_count.get() == 0 {

internal/compiler/llr/lower_to_item_tree.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -390,11 +390,17 @@ fn lower_sub_component(
390390

391391
crate::generator::handle_property_bindings_init(component, |e, p, binding| {
392392
let nr = NamedReference::new(e, p.clone());
393-
let prop = ctx.map_property_reference(&nr).local();
393+
let prop = ctx.map_property_reference(&nr);
394394

395395
if let Type::Function { .. } = nr.ty() {
396-
assert!(prop.sub_component_path.is_empty());
397-
let LocalMemberIndex::Function(function_index) = prop.reference else { unreachable!() };
396+
let MemberReference::Relative { parent_level, local_reference } = prop else {
397+
unreachable!()
398+
};
399+
assert!(parent_level == 0);
400+
assert!(local_reference.sub_component_path.is_empty());
401+
let LocalMemberIndex::Function(function_index) = local_reference.reference else {
402+
unreachable!()
403+
};
398404

399405
sub_component.functions[function_index].code =
400406
super::lower_expression::lower_expression(&binding.expression, &mut ctx);
@@ -454,7 +460,7 @@ fn lower_sub_component(
454460
if let Some(anim) = binding.animation.as_ref() {
455461
match super::lower_expression::lower_animation(anim, &mut ctx) {
456462
Animation::Static(anim) => {
457-
sub_component.animations.insert(prop, anim);
463+
sub_component.animations.insert(prop.local(), anim);
458464
}
459465
Animation::Transition(_) => {
460466
// Cannot set a property with a transition anyway
@@ -499,13 +505,12 @@ fn lower_sub_component(
499505

500506
crate::generator::for_each_const_properties(component, |elem, n| {
501507
let x = ctx.map_property_reference(&NamedReference::new(elem, n.clone()));
502-
let x = x.local();
503508
// ensure that all const properties have analysis
504509
sub_component.prop_analysis.entry(x.clone()).or_insert_with(|| PropAnalysis {
505510
property_init: None,
506511
analysis: get_property_analysis(elem, n),
507512
});
508-
sub_component.const_properties.push(x);
513+
sub_component.const_properties.push(x.local());
509514
});
510515

511516
sub_component.init_code = component
@@ -569,7 +574,7 @@ fn lower_sub_component(
569574
&tree_Expression::CodeBlock(exprs),
570575
&mut ctx,
571576
);
572-
(prop.local(), expr.into())
577+
(prop, expr.into())
573578
})
574579
.collect();
575580

internal/compiler/llr/pretty_print.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl PrettyPrinter<'_> {
6565
writeln!(
6666
self.writer,
6767
"{} <=> {}{}{};",
68-
DisplayLocalRef(p1, &ctx),
68+
DisplayPropertyRef(p1, &ctx),
6969
DisplayPropertyRef(p2, &ctx),
7070
if fields.is_empty() { "" } else { "." },
7171
fields.join(".")
@@ -76,7 +76,7 @@ impl PrettyPrinter<'_> {
7676
writeln!(
7777
self.writer,
7878
"{}: {};{}",
79-
DisplayLocalRef(p, &ctx),
79+
DisplayPropertyRef(p, &ctx),
8080
DisplayExpression(&init.expression.borrow(), &ctx),
8181
if init.is_constant { " /*const*/" } else { "" }
8282
)?
@@ -86,7 +86,7 @@ impl PrettyPrinter<'_> {
8686
writeln!(
8787
self.writer,
8888
"changed {} => {};",
89-
DisplayLocalRef(p, &ctx),
89+
DisplayPropertyRef(p, &ctx),
9090
DisplayExpression(&e.borrow(), &ctx),
9191
)?
9292
}

tests/cases/bindings/two_way_global.slint

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,37 @@
11
// Copyright © SixtyFPS GmbH <info@slint.dev>
22
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
33

4+
// See FIXME in the js test
5+
//ignore: live-preview
6+
47
export global G1 {
58
in-out property <int> data: 42;
9+
in-out property <string> string-prop;
610
}
711

812
export global G2 {
913
in-out property <int> data <=> G1.data;
1014
}
1115

12-
export component TestCase inherits Window {
16+
component SubComponent {
17+
in-out property <string> str <=> G1.string-prop;
18+
}
1319

20+
export component TestCase inherits Window {
1421
out property <int> d1: G1.data;
1522
out property <int> d2: G2.data;
16-
out property <bool> test: G2.data == 42;
23+
sc1 := SubComponent {
24+
str: "sc1";
25+
}
26+
sc2 := SubComponent {
27+
str: "sc2";
28+
}
29+
30+
in-out property <string> string-prop-alias <=> sc1.str;
31+
out property <int> string-prop-alias-changed: 0;
32+
changed string-prop-alias => { string-prop-alias-changed+=1; }
33+
34+
out property <bool> test: G2.data == 42 && sc1.str == sc2.str;
1735
}
1836

1937

@@ -37,6 +55,13 @@ assert_eq!(G2::get(&instance).get_data(), -1);
3755
assert_eq!(instance.get_d1(), -1);
3856
assert_eq!(instance.get_d2(), -1);
3957
58+
let string_prop = instance.global::<G1<'_>>().get_string_prop();
59+
assert!(string_prop == "sc1" || string_prop == "sc2");
60+
instance.set_string_prop_alias("xxx".into());
61+
let string_prop = instance.global::<G1<'_>>().get_string_prop();
62+
assert_eq!(string_prop, "xxx");
63+
slint_testing::mock_elapsed_time(1);
64+
assert_eq!(instance.get_string_prop_alias_changed(), 1);
4065
```
4166
4267
```cpp
@@ -57,6 +82,14 @@ assert_eq(instance.global<G1>().get_data(), -1);
5782
assert_eq(instance.global<G2>().get_data(), -1);
5883
assert_eq(instance.get_d1(), -1);
5984
assert_eq(instance.get_d2(), -1);
85+
86+
auto string_prop = instance.global<G1>().get_string_prop();
87+
assert(string_prop == "sc1" || string_prop == "sc2");
88+
instance.set_string_prop_alias("xxx");
89+
string_prop = instance.global<G1>().get_string_prop();
90+
assert_eq(string_prop, "xxx");
91+
slint_testing::mock_elapsed_time(1);
92+
assert_eq(instance.get_string_prop_alias_changed(), 1);
6093
```
6194
6295
```js
@@ -76,6 +109,17 @@ assert.equal(instance.G1.data, -1);
76109
assert.equal(instance.G2.data, -1);
77110
assert.equal(instance.d1, -1);
78111
assert.equal(instance.d2, -1);
112+
113+
// FIXME! This is not working with the interpreter
114+
/*
115+
let string_prop = instance.G1.string_prop;
116+
assert(string_prop == "sc1" || string_prop == "sc2");
117+
instance.G1.string_prop = "xxx";
118+
string_prop = instance.G1.string_prop;
119+
assert.equal(string_prop, "xxx");
120+
slintlib.private_api.mock_elapsed_time(1);
121+
assert.equal(instance.string_prop_alias_changed, 1);
122+
*/
79123
```
80124
81125
*/

0 commit comments

Comments
 (0)