Compiler: keep the layout debugging info next to its node

internal/compiler/object_tree.rs

@@ -538,7 +538,7 @@ impl Clone for PropertyAnimation {
                 property_analysis: e.property_analysis.clone(),
                 enclosing_component: e.enclosing_component.clone(),
                 repeated: None,
-                node: e.node.clone(),
+                debug: e.debug.clone(),
                 ..Default::default()
             }))
         }
@@ -650,22 +650,23 @@ pub struct Element {
     /// How many times the element was inlined
     pub inline_depth: i32,
 
-    /// The AST nodes, if available.
+    /// Debug information about this element.
+    ///
+    /// Contains the AST node if available, as well as wether this element was a layout that had
+    /// been lowered into a rectangle in the lower_layouts pass.
     /// There can be several in case of inlining or optimization (child merged into their parent).
+    ///
     /// The order in the list is first the parent, and then the removed children.
-    pub node: Vec<syntax_nodes::Element>,
+    pub debug: Vec<(syntax_nodes::Element, Option<crate::layout::Layout>)>,
-
-    /// This element was a layout that has been lowered to a Rectangle
-    pub layout: Option<crate::layout::Layout>,
 }
 
 impl Spanned for Element {
     fn span(&self) -> crate::diagnostics::Span {
-        self.node.first().map(|n| n.span()).unwrap_or_default()
+        self.debug.first().map(|n| n.0.span()).unwrap_or_default()
     }
 
     fn source_file(&self) -> Option<&crate::diagnostics::SourceFile> {
-        self.node.first().map(|n| &n.source_file)
+        self.debug.first().map(|n| &n.0.source_file)
     }
 }
 
@@ -899,7 +900,7 @@ impl Element {
         let mut r = Element {
             id,
             base_type,
-            node: vec![node.clone()],
+            debug: vec![(node.clone(), None)],
             is_legacy_syntax,
             ..Default::default()
         };
@@ -1633,9 +1634,9 @@ impl Element {
 
     /// Returns the element's name as specified in the markup, not normalized.
     pub fn original_name(&self) -> String {
-        self.node
+        self.debug
             .first()
-            .and_then(|n| n.child_token(parser::SyntaxKind::Identifier))
+            .and_then(|n| n.0.child_token(parser::SyntaxKind::Identifier))
             .map(|n| n.to_string())
             .unwrap_or_else(|| self.id.clone())
     }
@@ -2134,9 +2135,11 @@ pub fn visit_all_named_references_in_element(
     let mut layout_info_prop = std::mem::take(&mut elem.borrow_mut().layout_info_prop);
     layout_info_prop.as_mut().map(|(h, b)| (vis(h), vis(b)));
     elem.borrow_mut().layout_info_prop = layout_info_prop;
-    let mut layout = std::mem::take(&mut elem.borrow_mut().layout);
+    let mut debug = std::mem::take(&mut elem.borrow_mut().debug);
-    layout.as_mut().map(|l| l.visit_named_references(&mut vis));
+    for d in debug.iter_mut() {
-    elem.borrow_mut().layout = layout;
+        d.1.as_mut().map(|l| l.visit_named_references(&mut vis));
+    }
+    elem.borrow_mut().debug = debug;
 
     let mut accessibility_props = std::mem::take(&mut elem.borrow_mut().accessibility_props);
     accessibility_props.0.iter_mut().for_each(|(_, x)| vis(x));

internal/compiler/passes/ensure_window.rs

@@ -51,8 +51,8 @@ pub fn ensure_window(
         is_flickable_viewport: false,
         item_index: Default::default(),
         item_index_of_first_children: Default::default(),
-        node: std::mem::take(&mut win_elem_mut.node),
+        debug: std::mem::take(&mut win_elem_mut.debug),
-        layout: win_elem_mut.layout.clone(),
+
         inline_depth: 0,
         is_legacy_syntax: false,
     };

internal/compiler/passes/inlining.rs

@@ -130,7 +130,7 @@ fn inline_element(
     }
 
     elem_mut.children = new_children;
-    elem_mut.node.extend_from_slice(&inlined_component.root_element.borrow().node);
+    elem_mut.debug.extend_from_slice(&inlined_component.root_element.borrow().debug);
 
     if let ElementType::Component(c) = &mut elem_mut.base_type {
         if c.parent_element.upgrade().is_some() {
@@ -264,8 +264,7 @@ fn duplicate_element_with_mapping(
             .collect(),
         repeated: elem.repeated.clone(),
         is_component_placeholder: elem.is_component_placeholder,
-        node: elem.node.clone(),
+        debug: elem.debug.clone(),
-        layout: elem.layout.clone(),
         enclosing_component: Rc::downgrade(root_component),
         states: elem.states.clone(),
         transitions: elem

internal/compiler/passes/lower_component_container.rs

@@ -36,7 +36,7 @@ fn process_component_container(element: &ElementRc, empty_type: &ElementType) {
     let embedded_element = Element::make_rc(Element {
         base_type: empty_type.clone(),
         id: elem.id.clone(),
-        node: elem.node.clone(),
+        debug: elem.debug.clone(),
         enclosing_component: elem.enclosing_component.clone(),
         default_fill_parent: (true, true),
         is_legacy_syntax: elem.is_legacy_syntax,

internal/compiler/passes/lower_layout.rs

@@ -246,7 +246,9 @@ fn lower_grid_layout(
     );
     grid_layout_element.borrow_mut().layout_info_prop =
         Some((layout_info_prop_h, layout_info_prop_v));
-    grid_layout_element.borrow_mut().layout = Some(Layout::GridLayout(grid));
+    if let Some(d) = grid_layout_element.borrow_mut().debug.last_mut() {
+        d.1 = Some(Layout::GridLayout(grid));
+    }
 }
 
 impl GridLayout {
@@ -434,7 +436,9 @@ fn lower_box_layout(
         .into(),
     );
     layout_element.borrow_mut().layout_info_prop = Some((layout_info_prop_h, layout_info_prop_v));
-    layout_element.borrow_mut().layout = Some(Layout::BoxLayout(layout));
+    if let Some(d) = layout_element.borrow_mut().debug.last_mut() {
+        d.1 = Some(Layout::BoxLayout(layout));
+    }
 }
 
 fn lower_dialog_layout(
@@ -645,7 +649,9 @@ fn lower_dialog_layout(
         .into(),
     );
     dialog_element.borrow_mut().layout_info_prop = Some((layout_info_prop_h, layout_info_prop_v));
-    dialog_element.borrow_mut().layout = Some(Layout::GridLayout(grid));
+    if let Some(d) = dialog_element.borrow_mut().debug.last_mut() {
+        d.1 = Some(Layout::GridLayout(grid));
+    }
 }
 
 struct CreateLayoutItemResult {

internal/compiler/passes/optimize_useless_rectangles.rs

@@ -22,7 +22,7 @@ pub fn optimize_useless_rectangles(root_component: &Rc<Component>) {
             }
 
             parent.children.extend(std::mem::take(&mut elem.borrow_mut().children));
-            parent.node.extend(std::mem::take(&mut elem.borrow_mut().node));
+            parent.debug.extend(std::mem::take(&mut elem.borrow_mut().debug));
 
             let enclosing = parent.enclosing_component.upgrade().unwrap();
 

internal/compiler/passes/repeater_component.rs

@@ -36,8 +36,7 @@ fn create_repeater_components(component: &Rc<Component>) {
                 named_references: Default::default(),
                 repeated: None,
                 is_component_placeholder: false,
-                node: elem.node.clone(),
+                debug: elem.debug.clone(),
-                layout: elem.layout.clone(),
                 enclosing_component: Default::default(),
                 states: std::mem::take(&mut elem.states),
                 transitions: std::mem::take(&mut elem.transitions),

internal/interpreter/highlight.rs

@@ -126,7 +126,11 @@ fn fill_highlight_data(
         let size = geometry.size;
 
         if values.kind.is_none() {
-            values.kind = if element.borrow().layout.is_some() {
+            // FIXME: this visualization is misleading because it will highlight as a layout any
+            // optimized rectangle within a layout or parent of a layout.
+            // Example: `foo := Rectangle { lay := SomeLayout { ... } } ` lay will be optimized into foo
+            // and so both foo and lay will be considered as layout (assuming SomeLayout inherits from a layout)
+            values.kind = if element.borrow().debug.iter().any(|d| d.1.is_some()) {
                 Some(ComponentKind::Layout)
             } else {
                 Some(ComponentKind::Element)
@@ -147,7 +151,7 @@ fn find_element_at_offset(component: &Rc<Component>, path: &Path, offset: u32) -
             if elem.borrow().repeated.is_some() {
                 return;
             }
-            for node in elem.borrow().node.iter().filter_map(|n| n.QualifiedName()) {
+            for node in elem.borrow().debug.iter().filter_map(|n| n.0.QualifiedName()) {
                 if node.source_file.path() == path && node.text_range().contains(offset.into()) {
                     result.push(elem.clone());
                 }

tools/lsp/language.rs

@@ -567,11 +567,12 @@ pub async fn set_binding_command(
             })?;
 
         let node_range = map_node(
-            element
+            &element
                 .borrow()
-                .node
+                .debug
                 .first()
-                .ok_or("The element was found, but had no range defined!")?,
+                .ok_or("The element was found, but had no range defined!")?
+                .0,
         )
         .ok_or("Failed to map node")?;
 
@@ -650,11 +651,12 @@ pub async fn remove_binding_command(
             })?;
 
         let node_range = map_node(
-            element
+            &element
                 .borrow()
-                .node
+                .debug
                 .first()
-                .ok_or("The element was found, but had no range defined!")?,
+                .ok_or("The element was found, but had no range defined!")?
+                .0,
         )
         .ok_or("Failed to map node")?;
 
@@ -802,10 +804,9 @@ fn get_document_and_offset<'a>(
 fn element_contains(element: &i_slint_compiler::object_tree::ElementRc, offset: u32) -> bool {
     element
         .borrow()
-        .node
+        .debug
-        .first()
+        .iter()
-        .and_then(|n| n.parent())
+        .any(|n| n.0.parent().map_or(false, |n| n.text_range().contains(offset.into())))
-        .map_or(false, |n| n.text_range().contains(offset.into()))
 }
 
 pub fn element_at_position(
@@ -1146,7 +1147,7 @@ fn get_document_symbols(
         .iter()
         .filter_map(|c| {
             let root_element = c.root_element.borrow();
-            let element_node = root_element.node.first()?;
+            let element_node = &root_element.debug.first()?.0;
             let component_node = syntax_nodes::Component::new(element_node.parent()?)?;
             let selection_range = map_node(&component_node.DeclaredIdentifier())?;
             if c.id.is_empty() {
@@ -1196,7 +1197,7 @@ fn get_document_symbols(
             .iter()
             .filter_map(|child| {
                 let e = child.borrow();
-                let element_node = e.node.first()?;
+                let element_node = &e.debug.first()?.0;
                 let sub_element_node = element_node.parent()?;
                 debug_assert_eq!(sub_element_node.kind(), SyntaxKind::SubElement);
                 Some(DocumentSymbol {
@@ -1233,7 +1234,7 @@ fn get_code_lenses(
         // Handle preview lens
         r.extend(inner_components.iter().filter(|c| !c.is_global()).filter_map(|c| {
             Some(CodeLens {
-                range: map_node(c.root_element.borrow().node.first()?)?,
+                range: map_node(&c.root_element.borrow().debug.first()?.0)?,
                 command: Some(create_show_preview_command(true, &text_document.uri, c.id.as_str())),
                 data: None,
             })

tools/lsp/language/goto.rs

@@ -37,7 +37,7 @@ pub fn goto_definition(
                     let doc = document_cache.documents.get_document(node.source_file.path())?;
                     match doc.local_registry.lookup_element(&qual.to_string()) {
                         Ok(ElementType::Component(c)) => {
-                            goto_node(c.root_element.borrow().node.first()?)
+                            goto_node(&c.root_element.borrow().debug.first()?.0)
                         }
                         _ => None,
                     }
@@ -68,7 +68,7 @@ pub fn goto_definition(
                         LookupResult::Expression {
                             expression: Expression::ElementReference(e),
                             ..
-                        } => e.upgrade()?.borrow().node.first()?.clone().into(),
+                        } => e.upgrade()?.borrow().debug.first()?.0.clone().into(),
                         LookupResult::Expression {
                             expression:
                                 Expression::CallbackReference(nr, _)
@@ -107,7 +107,9 @@ pub fn goto_definition(
             let doc = document_cache.documents.get_document(node.source_file.path())?;
             let imp_name = i_slint_compiler::typeloader::ImportedName::from_node(n);
             return match doc.local_registry.lookup_element(&imp_name.internal_name) {
-                Ok(ElementType::Component(c)) => goto_node(c.root_element.borrow().node.first()?),
+                Ok(ElementType::Component(c)) => {
+                    goto_node(&c.root_element.borrow().debug.first()?.0)
+                }
                 _ => None,
             };
         } else if let Some(n) = syntax_nodes::ImportSpecifier::new(node.clone()) {

tools/lsp/language/properties.rs

@@ -234,7 +234,7 @@ fn find_expression_range(
 }
 
 fn find_property_binding_offset(element: &Element, property_name: &str) -> Option<u32> {
-    let element_range = element.node.first()?.text_range();
+    let element_range = element.debug.first()?.0.text_range();
 
     if let Some(v) = element.bindings.get(property_name) {
         if let Some(span) = &v.borrow().span {
@@ -252,7 +252,7 @@ fn find_property_binding_offset(element: &Element, property_name: &str) -> Optio
 }
 
 fn insert_property_definitions(element: &Element, properties: &mut Vec<PropertyInformation>) {
-    if let Some(element_node) = element.node.first() {
+    if let Some((element_node, _)) = element.debug.first() {
         for prop_info in properties {
             if let Some(offset) = find_property_binding_offset(element, prop_info.name.as_str()) {
                 prop_info.defined_at = find_expression_range(element_node, offset);
@@ -390,7 +390,7 @@ fn get_properties(element: &ElementRc) -> Vec<PropertyInformation> {
 
 fn find_block_range(element: &ElementRc) -> Option<lsp_types::Range> {
     let element = element.borrow();
-    let node = element.node.first()?;
+    let node = &element.debug.first()?.0;
 
     let open_brace = node.child_token(SyntaxKind::LBrace)?;
     let close_brace = node.child_token(SyntaxKind::RBrace)?;
@@ -407,7 +407,7 @@ fn get_element_information(element: &ElementRc) -> ElementInformation {
     ElementInformation {
         id: e.id.clone(),
         type_name: e.base_type.to_string(),
-        range: e.node.first().and_then(|n| map_node(n)),
+        range: e.debug.first().and_then(|n| map_node(&n.0)),
     }
 }
 
@@ -622,7 +622,7 @@ pub(crate) fn set_binding(
 
     let new_expression_type = {
         let element = element.borrow();
-        if let Some(node) = element.node.first() {
+        if let Some((node, _)) = &element.debug.first() {
             let expr_context_info =
                 ExpressionContextInfo::new(node.clone(), property_name.to_string(), false);
             with_property_lookup_ctx(document_cache, &expr_context_info, |ctx| {
@@ -734,10 +734,10 @@ pub(crate) fn remove_binding(
     property_name: &str,
 ) -> Result<lsp_types::WorkspaceEdit> {
     let element = element.borrow();
-    let source_file = element.node.first().and_then(|n| n.source_file());
+    let source_file = &element.debug.first().and_then(|n| n.0.source_file());
 
     let range = find_property_binding_offset(&element, property_name)
-        .and_then(|offset| element.node.first()?.token_at_offset(offset.into()).right_biased())
+        .and_then(|offset| element.debug.first()?.0.token_at_offset(offset.into()).right_biased())
         .and_then(|token| {
             for ancestor in token.parent_ancestors() {
                 if (ancestor.kind() == SyntaxKind::Binding)

tools/lsp/preview.rs

@@ -422,12 +422,11 @@ pub fn load_preview(preview_component: PreviewComponent) {
                         .element_at_source_code_position(&se.path, se.offset)
                         .first()
                     {
-                        if let Some(node) = element
+                        if let Some((node, _)) = element
                             .borrow()
-                            .node
+                            .debug
                             .iter()
-                            .filter(|n| !crate::common::is_element_node_ignored(n))
+                            .find(|n| !crate::common::is_element_node_ignored(&n.0))
-                            .next()
                         {
                             let sf = &node.source_file;
                             let pos = map_position(sf, se.offset.into());
@@ -544,7 +543,14 @@ pub fn highlight(url: Option<Url>, offset: u32) {
             };
             let elements = component_instance.element_at_source_code_position(&path, offset);
             if let Some(e) = elements.first() {
-                let is_layout = e.borrow().layout.is_some();
+                let is_layout = e
+                    .borrow()
+                    .debug
+                    .iter()
+                    .find(|(node, _)| {
+                        node.text_range().contains(offset.into()) && node.source_file.path() == path
+                    })
+                    .map_or(false, |d| d.1.is_some());
                 element_selection::select_element_at_source_code_position(
                     path, offset, is_layout, None, false,
                 );

tools/lsp/preview/debug.rs

@@ -168,7 +168,7 @@ fn recurse_into_element(state: &mut State, element: &ElementRc) -> (usize, Vec<S
 
 fn add_element_node(state: &mut State, element: &ElementRc, node_number: usize) {
     let e = element.borrow();
-    let layout = if e.layout.is_some() { ",shape = box" } else { "" };
+    let layout = if e.debug.iter().any(|d| d.1.is_some()) { ",shape = box" } else { "" };
     let repeated = if e.repeated.is_some() { ",color = blue" } else { "" };
     let component = if matches!(e.base_type, i_slint_compiler::langtype::ElementType::Component(_))
     {

tools/lsp/preview/drop_location.rs

@@ -25,22 +25,21 @@ fn find_drop_location(
     let (node_index, target_element) = elements.iter().find_map(|sc| {
         sc.element
             .borrow()
-            .node
+            .debug
             .iter()
-            .enumerate()
+            .position(|d| !crate::common::is_element_node_ignored(&d.0))
-            .filter(|(_, n)| !crate::common::is_element_node_ignored(n))
+            .map(|i| (i, sc.element.clone()))
-            .next()
-            .map(|(i, _)| (i, sc.element.clone()))
     })?;
 
     let insertion_position = {
         let elem = target_element.borrow();
 
-        if elem.layout.is_some() {
+        let (node, layout) = elem.debug.get(node_index)?;
+
+        if layout.is_some() {
             return None;
         }
 
-        let node = elem.node.get(node_index)?;
         let last_token = crate::util::last_non_ws_token(node)?;
 
         let url = lsp_types::Url::from_file_path(node.source_file.path()).ok()?;

tools/lsp/preview/element_selection.rs

@@ -42,19 +42,16 @@ fn self_or_embedded_component_root(element: &ElementRc) -> ElementRc {
 
 fn lsp_element_position(element: &ElementRc) -> Option<(String, lsp_types::Range)> {
     let e = element.borrow();
-    let location = e
+    let location = e.debug.iter().find(|e| !crate::common::is_element_node_ignored(&e.0)).and_then(
-        .node
+        |(n, _)| {
-        .iter()
-        .filter(|e| !crate::common::is_element_node_ignored(*e))
-        .next()
-        .and_then(|n| {
             n.parent()
                 .filter(|p| p.kind() == i_slint_compiler::parser::SyntaxKind::SubElement)
                 .map_or_else(
                     || Some(n.source_file.text_size_to_file_line_column(n.text_range().start())),
                     |p| Some(p.source_file.text_size_to_file_line_column(p.text_range().start())),
                 )
-        });
+        },
+    );
     location.map(|(f, sl, sc, el, ec)| {
         use lsp_types::{Position, Range};
         let start = Position::new((sl as u32).saturating_sub(1), (sc as u32).saturating_sub(1));
@@ -132,7 +129,8 @@ fn select_element(
             component_instance,
             path,
             offset,
-            selected_element.borrow().layout.is_some(),
+            // FIXME: need to check which one of the node this refer to to know if this is a layout
+            selected_element.borrow().debug.iter().any(|d| d.1.is_some()),
             position,
             false, // We update directly;-)
         );
@@ -146,16 +144,14 @@ fn select_element(
 }
 
 fn element_offset(element: &ElementRc) -> Option<(PathBuf, u32)> {
-    let Some(node) = element.borrow().node.first().cloned() else {
+    let node = element.borrow().debug.first()?.0.clone();
-        return None;
-    };
     let path = node.source_file.path().to_path_buf();
     let offset = node.text_range().start().into();
     Some((path, offset))
 }
 
 fn element_source_range(element: &ElementRc) -> Option<(SourceFile, TextRange)> {
-    let node = element.borrow().node.first().cloned()?;
+    let node = element.borrow().debug.first()?.0.clone();
     let source_file = node.source_file.clone();
     let range = node.text_range();
     Some((source_file, range))
@@ -164,7 +160,7 @@ fn element_source_range(element: &ElementRc) -> Option<(SourceFile, TextRange)>
 // Return the real root element, skipping any WindowElement that got added
 pub fn root_element(component_instance: &ComponentInstance) -> ElementRc {
     let root_element = component_instance.definition().root_component().root_element.clone();
-    if !root_element.borrow().node.is_empty() {
+    if !root_element.borrow().debug.is_empty() {
         return root_element;
     }
     let child = root_element.borrow().children.first().cloned();
@@ -192,10 +188,10 @@ impl SelectionCandidate {
 
     pub fn is_builtin(&self) -> bool {
         let elem = self.element.borrow();
-        let Some(node) = elem.node.first() else {
+        let Some(node) = elem.debug.first() else {
             return true;
         };
-        let Some(sf) = node.source_file() else {
+        let Some(sf) = node.0.source_file() else {
             return true;
         };
         sf.path().starts_with("builtin:/")

tools/lsp/util.rs

@@ -60,8 +60,7 @@ pub fn last_non_ws_token(node: &SyntaxNode) -> Option<SyntaxToken> {
 // Find the indentation of the element itself as well as the indentation of properties inside the
 // element. Returns the element indent followed by the block indent
 pub fn find_element_indent(element: &ElementRc) -> Option<String> {
-    let mut token =
+    let mut token = element.borrow().debug.first()?.0.first_token()?.prev_token();
-        element.borrow().node.first().and_then(|n| n.first_token()).and_then(|t| t.prev_token());
     while let Some(t) = token {
         if t.kind() == SyntaxKind::Whitespace && t.text().contains('\n') {
             return t.text().split('\n').last().map(|s| s.to_owned());
@@ -180,7 +179,7 @@ pub fn with_property_lookup_ctx<R>(
         loop {
             scope.push(it.clone());
             if let Some(c) = it.clone().borrow().children.iter().find(|c| {
-                c.borrow().node.first().map_or(false, |n| n.text_range().contains(offset))
+                c.borrow().debug.first().map_or(false, |n| n.0.text_range().contains(offset))
             }) {
                 it = c.clone();
             } else {

tools/updater/main.rs

@@ -222,7 +222,7 @@ fn visit_node(
                     .borrow()
                     .children
                     .iter()
-                    .find(|c| c.borrow().node.first().map_or(false, |n| n.node == node.node))
+                    .find(|c| c.borrow().debug.first().map_or(false, |n| n.0.node == node.node))
                     .cloned()
             } else if let Some(parent_co) = &state.current_component {
                 if node.parent().map_or(false, |n| n.kind() == SyntaxKind::Component) {