From 94af4d28321396db909476215c2c321d265242c7 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <olivier.goffart@slint.dev>
Date: Tue, 4 Jun 2024 13:34:47 +0200
Subject: [PATCH] Don't optimize array

They are model and if the model is copied, the changes in model need to
apply to all copies.

Fixes #5249
---
 internal/compiler/expression_tree.rs          |  5 +-
 .../llr/optim_passes/inline_expressions.rs    |  4 +-
 .../cases/models/indirect_model_changes.slint | 61 +++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 tests/cases/models/indirect_model_changes.slint

diff --git a/internal/compiler/expression_tree.rs b/internal/compiler/expression_tree.rs
index af4ed1fdc8f..0e5afa25520 100644
--- a/internal/compiler/expression_tree.rs
+++ b/internal/compiler/expression_tree.rs
@@ -1069,7 +1069,10 @@ impl Expression {
             }
             Expression::BinaryExpression { lhs, rhs, .. } => lhs.is_constant() && rhs.is_constant(),
             Expression::UnaryOp { sub, .. } => sub.is_constant(),
-            Expression::Array { values, .. } => values.iter().all(Expression::is_constant),
+            // Array will turn into model, and they can't be considered as constant if the model
+            // is used and the model is changed. CF issue #5249
+            //Expression::Array { values, .. } => values.iter().all(Expression::is_constant),
+            Expression::Array { .. } => false,
             Expression::Struct { values, .. } => values.iter().all(|(_, v)| v.is_constant()),
             Expression::PathData(data) => match data {
                 Path::Elements(elements) => elements
diff --git a/internal/compiler/llr/optim_passes/inline_expressions.rs b/internal/compiler/llr/optim_passes/inline_expressions.rs
index 444a13d51dc..4be46e8dce9 100644
--- a/internal/compiler/llr/optim_passes/inline_expressions.rs
+++ b/internal/compiler/llr/optim_passes/inline_expressions.rs
@@ -39,7 +39,9 @@ fn expression_cost(exp: &Expression, ctx: &EvaluationContext) -> isize {
         Expression::UnaryOp { .. } => 1,
         Expression::ImageReference { .. } => 1,
         Expression::Condition { .. } => 10,
-        Expression::Array { .. } => ALLOC_COST,
+        // Never inline an array because it is a model and when shared it needs to keep its identity
+        // (cf #5249)  (otherwise it would be `ALLOC_COST`)
+        Expression::Array { .. } => return isize::MAX,
         Expression::Struct { .. } => 1,
         Expression::EasingCurve(_) => 1,
         Expression::LinearGradient { .. } => ALLOC_COST,
diff --git a/tests/cases/models/indirect_model_changes.slint b/tests/cases/models/indirect_model_changes.slint
new file mode 100644
index 00000000000..e4335e34ae6
--- /dev/null
+++ b/tests/cases/models/indirect_model_changes.slint
@@ -0,0 +1,61 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+component IndirectChange {
+    in property <[int]> mod;
+    property <[int]> private: mod;
+
+    init => {
+        private[0] += 1;
+    }
+}
+
+export component TestCase {
+
+    property <[int]> m1: [5];
+    property <[int]> m2: [8];
+    property <[int]> indirect: m2;
+
+    public function up() {
+        indirect[0] += 10;
+    }
+
+    callback up2();
+    up2 => {up()}
+
+    IndirectChange { mod: m1; }
+
+    out property <int> t1: m1[0];
+    out property <int> t2: m2[0];
+
+    out property <bool> test: t1 == 5+1 && t2 == 8;
+}
+
+/*
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert_eq(instance.get_t1(), 6);
+assert(instance.get_test());
+instance.invoke_up();
+assert_eq(instance.get_t2(), 18);
+
+```
+
+
+```rust
+let instance = TestCase::new().unwrap();
+assert_eq!(instance.get_t1(), 6);
+assert!(instance.get_test());
+instance.invoke_up();
+assert_eq!(instance.get_t2(), 18);
+```
+
+```js
+var instance = new slint.TestCase({});
+assert.equal(instance.t1, 6);
+assert(instance.test);
+instance.up2();
+assert.equal(instance.t2, 18);
+```
+*/