diff --git a/internal/compiler/object_tree.rs b/internal/compiler/object_tree.rs index 6105ee575d3..656767036a9 100644 --- a/internal/compiler/object_tree.rs +++ b/internal/compiler/object_tree.rs @@ -944,9 +944,12 @@ pub fn pretty_print( } } for (name, expr) in &e.bindings { - let expr = expr.borrow(); indent!(); write!(f, "{name}: ")?; + let Ok(expr) = expr.try_borrow() else { + writeln!(f, "")?; + continue; + }; expression_tree::pretty_print(f, &expr.expression)?; if expr.analysis.as_ref().is_some_and(|a| a.is_const) { write!(f, "/*const*/")?; diff --git a/internal/compiler/passes/lower_menus.rs b/internal/compiler/passes/lower_menus.rs index e7289f4a556..b65a8402bd9 100644 --- a/internal/compiler/passes/lower_menus.rs +++ b/internal/compiler/passes/lower_menus.rs @@ -248,8 +248,6 @@ fn process_context_menu( .clone() .into(); - context_menu_elem.borrow_mut().base_type = components.context_menu_internal.clone(); - let mut menu_elem = None; context_menu_elem.borrow_mut().children.retain(|x| { if x.borrow().base_type == menu_element_type { @@ -282,9 +280,10 @@ fn process_context_menu( } let children = std::mem::take(&mut menu_elem.borrow_mut().children); - let c = lower_menu_items(context_menu_elem, children, components); + let c = lower_menu_items(context_menu_elem, children, components, diag); let item_tree_root = Expression::ElementReference(Rc::downgrade(&c.root_element)); + context_menu_elem.borrow_mut().base_type = components.context_menu_internal.clone(); for (name, _) in &components.context_menu_internal.property_list() { if let Some(decl) = context_menu_elem.borrow().property_declarations.get(name) { diag.push_error(format!("Cannot re-define internal property '{name}'"), &decl.node); @@ -341,9 +340,8 @@ fn process_window( no_native_menu: bool, diag: &mut BuildDiagnostics, ) -> bool { - let mut window = win.borrow_mut(); let mut menu_bar = None; - window.children.retain(|x| { + win.borrow_mut().children.retain(|x| { if matches!(&x.borrow().base_type, ElementType::Builtin(b) if b.name == "MenuBar") { if menu_bar.is_some() { diag.push_error("Only one MenuBar is allowed in a Window".into(), &*x.borrow()); @@ -370,7 +368,7 @@ fn process_window( // Lower MenuItem's into a tree root let children = std::mem::take(&mut menu_bar.borrow_mut().children); - let c = lower_menu_items(&menu_bar, children, components); + let c = lower_menu_items(&menu_bar, children, components, diag); let item_tree_root = Expression::ElementReference(Rc::downgrade(&c.root_element)); if !no_native_menu { @@ -393,6 +391,7 @@ fn process_window( }; } + let mut window = win.borrow_mut(); let menubar_impl = Element { id: format_smolstr!("{}-menulayout", window.id), base_type: components.menubar_impl.clone(), @@ -517,6 +516,7 @@ fn lower_menu_items( parent: &ElementRc, children: Vec, components: &UsefulMenuComponents, + diag: &mut BuildDiagnostics, ) -> Rc { let component = Rc::new_cyclic(|component_weak| { let root_element = Rc::new(RefCell::new(Element { @@ -556,13 +556,17 @@ fn lower_menu_items( ..Default::default() } }); - parent - .borrow() - .enclosing_component - .upgrade() - .unwrap() - .menu_item_tree - .borrow_mut() - .push(component.clone()); + let enclosing = parent.borrow().enclosing_component.upgrade().unwrap(); + + super::lower_popups::check_no_reference_to_popup( + parent, + &enclosing, + &Rc::downgrade(&component), + &NamedReference::new(parent, SmolStr::new_static("x")), + diag, + ); + + enclosing.menu_item_tree.borrow_mut().push(component.clone()); + component } diff --git a/internal/compiler/passes/lower_popups.rs b/internal/compiler/passes/lower_popups.rs index 094c47a6875..0a35318a8c4 100644 --- a/internal/compiler/passes/lower_popups.rs +++ b/internal/compiler/passes/lower_popups.rs @@ -112,10 +112,6 @@ fn lower_popup_window( parent_cip.insertion_index -= 1; } - if matches!(popup_window_element.borrow().base_type, ElementType::Builtin(_)) { - popup_window_element.borrow_mut().base_type = window_type.clone(); - } - let map_close_on_click_value = |b: &BindingExpression| { let Expression::BoolLiteral(v) = super::ignore_debug_hooks(&b.expression) else { assert!(diag.has_errors()); @@ -213,28 +209,11 @@ fn lower_popup_window( popup_mut.geometry_props.as_mut().unwrap().y = dummy2; } - // Throw error when accessing the popup from outside - // FIXME: - // - the span is the span of the PopupWindow, that's wrong, we should have the span of the reference - // - There are other object reference than in the NamedReference - // - Maybe this should actually be allowed - visit_all_named_references(&parent_component, &mut |nr| { - let element = &nr.element(); - if check_element(element, &weak, diag, popup_window_element) { - // just set it to whatever is a valid NamedReference, otherwise we'll panic later - *nr = coord_x.clone(); - } - }); - visit_all_expressions(&parent_component, |exp, _| { - exp.visit_recursive_mut(&mut |exp| { - if let Expression::ElementReference(element) = exp { - let elem = element.upgrade().unwrap(); - if !Rc::ptr_eq(&elem, popup_window_element) { - check_element(&elem, &weak, diag, popup_window_element); - } - } - }); - }); + check_no_reference_to_popup(popup_window_element, &parent_component, &weak, &coord_x, diag); + + if matches!(popup_window_element.borrow().base_type, ElementType::Builtin(_)) { + popup_window_element.borrow_mut().base_type = window_type.clone(); + } super::focus_handling::call_focus_on_init(&popup_comp); @@ -251,15 +230,63 @@ fn report_const_error(prop: &str, span: &Option, diag: &mut Buil diag.push_error(format!("The {prop} property only supports constants at the moment"), span); } +/// Throw error when accessing the popup from outside +// FIXME: +// - the span is the span of the PopupWindow, that's wrong, we should have the span of the reference +// - There are other object reference than in the NamedReference +// - Maybe this should actually be allowed +pub fn check_no_reference_to_popup( + popup_window_element: &ElementRc, + parent_component: &Rc, + new_weak: &Weak, + random_valid_ref: &NamedReference, + diag: &mut BuildDiagnostics, +) { + visit_all_named_references(parent_component, &mut |nr| { + let element = &nr.element(); + if check_element(element, new_weak, diag, popup_window_element, nr.name()) { + // just set it to whatever is a valid NamedReference, otherwise we'll panic later + *nr = random_valid_ref.clone(); + } + }); + visit_all_expressions(parent_component, |exp, _| { + exp.visit_recursive_mut(&mut |exp| { + if let Expression::ElementReference(element) = exp { + let elem = element.upgrade().unwrap(); + if !Rc::ptr_eq(&elem, popup_window_element) { + check_element(&elem, new_weak, diag, popup_window_element, &""); + } + } + }); + }); +} + fn check_element( element: &ElementRc, popup_comp: &Weak, diag: &mut BuildDiagnostics, popup_window_element: &ElementRc, + prop_name: &str, ) -> bool { if Weak::ptr_eq(&element.borrow().enclosing_component, popup_comp) { + let element_name = popup_window_element + .borrow() + .builtin_type() + .map(|t| t.name.clone()) + .unwrap_or_else(|| SmolStr::new_static("PopupWindow")); + let id = element.borrow().id.clone(); + let what = if prop_name.is_empty() { + if id.is_empty() { "something".into() } else { format!("element '{id}'") } + } else { + if id.is_empty() { + format!("property or callback '{prop_name}'") + } else { + format!("property or callback '{id}.{prop_name}'") + } + }; + diag.push_error( - "Cannot access the inside of a PopupWindow from enclosing component".into(), + format!("Cannot access {what} inside of a {element_name} from enclosing component"), &*popup_window_element.borrow(), ); true diff --git a/internal/compiler/tests/syntax/basic/popup.slint b/internal/compiler/tests/syntax/basic/popup.slint index b251881a144..05dd80d36fb 100644 --- a/internal/compiler/tests/syntax/basic/popup.slint +++ b/internal/compiler/tests/syntax/basic/popup.slint @@ -9,7 +9,7 @@ component Foo { component Issue5852 { p := PopupWindow { -// > +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 + +export component A { + cb := ContextMenuArea { +// > { + inner-menu.title = "Hi"; + } + } +} + diff --git a/internal/compiler/tests/syntax/elements/menubar4.slint b/internal/compiler/tests/syntax/elements/menubar4.slint new file mode 100644 index 00000000000..a9688e3a655 --- /dev/null +++ b/internal/compiler/tests/syntax/elements/menubar4.slint @@ -0,0 +1,15 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 + +export component B inherits Window { + callback activate <=> item.activated; + MenuBar { +// >