Fix stack overflow in purity check

Fix #5220
slint-ui · May 29, 2024 · 8221e2e · 8221e2e
1 parent bc8e18e
commit 8221e2e
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 21 deletions.
diff --git a/internal/compiler/passes/purity_check.rs b/internal/compiler/passes/purity_check.rs
@@ -1,8 +1,10 @@
 // Copyright © SixtyFPS GmbH <info@slint.dev>
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.2 OR LicenseRef-Slint-commercial
 
+use std::collections::HashSet;
+
 use crate::diagnostics::BuildDiagnostics;
-use crate::expression_tree::Expression;
+use crate::expression_tree::{Expression, NamedReference};
 
 /// Check that pure expression only call pure functions
 pub fn purity_check(doc: &crate::object_tree::Document, diag: &mut BuildDiagnostics) {
@@ -21,11 +23,11 @@ pub fn purity_check(doc: &crate::object_tree::Document, diag: &mut BuildDiagnost
                         if lookup.declared_pure.unwrap_or(false)
                             || lookup.property_type.is_property_type()
                         {
-                            ensure_pure(expr, Some(diag), level);
+                            ensure_pure(expr, Some(diag), level, &mut Default::default());
                         }
                     } else {
                         // model expression must be pure
-                        ensure_pure(expr, Some(diag), level);
+                        ensure_pure(expr, Some(diag), level, &mut Default::default());
                     };
                 })
             },
@@ -37,6 +39,7 @@ fn ensure_pure(
     expr: &Expression,
     mut diag: Option<&mut BuildDiagnostics>,
     level: crate::diagnostics::DiagnosticLevel,
+    recursion_test: &mut HashSet<NamedReference>,
 ) -> bool {
     let mut r = true;
     expr.visit_recursive(&mut |e| match e {
@@ -66,25 +69,29 @@ fn ensure_pure(
                     r = false;
                 }
                 None => {
-                    if !ensure_pure(
-                        &nr.element()
-                            .borrow()
-                            .bindings
-                            .get(nr.name())
-                            .expect("private function must be local and defined")
-                            .borrow()
-                            .expression,
-                        None,
-                        level,
-                    ) {
-                        if let Some(diag) = diag.as_deref_mut() {
-                            diag.push_diagnostic(
-                                format!("Call of impure function '{}'", nr.name()),
-                                node,
-                                level,
-                            );
+                    if recursion_test.insert(nr.clone()) {
+                        if !ensure_pure(
+                            &nr.element()
+                                .borrow()
+                                .bindings
+                                .get(nr.name())
+                                .expect("private function must be local and defined")
+                                .borrow()
+                                .expression,
+                            None,
+                            level,
+                            recursion_test,
+                        ) {
+                            if let Some(diag) = diag.as_deref_mut() {
+                                diag.push_diagnostic(
+                                    format!("Call of impure function '{}'", nr.name()),
+                                    node,
+                                    level,
+                                );
+                            }
+                            r = false;
                         }
-                        r = false;
+                        recursion_test.remove(&nr);
                     }
                 }
             }

diff --git a/internal/compiler/tests/syntax/functions/functions_purity_recursive_5220.slint b/internal/compiler/tests/syntax/functions/functions_purity_recursive_5220.slint
@@ -0,0 +1,25 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.2 OR LicenseRef-Slint-commercial
+
+export component App inherits Window{
+    background: blue;
+    //          ^error{'blue' must be called}
+    //          ^^error{Cannot convert function\(\) -> color to brush}
+    function blue()->color {
+//  ^error{The binding for the property 'blue' is part of a binding loop}
+//  ^^error{Cannot convert function\(\) -> color to color}
+
+       blue
+//     ^error{'blue' must be called}
+    }
+
+
+
+    in property <int> abc: get_abc1();
+    function get_abc1() -> int { return get_abc2(); }
+//  ^error{The binding for the property 'get-abc1' is part of a binding loop}
+    function get_abc2() -> int { return get_abc1(); }
+//  ^error{The binding for the property 'get-abc2' is part of a binding loop}
+
+}
+