From 07e58ec5017d9827c08b1d9ea5ca121e79e10786 Mon Sep 17 00:00:00 2001
From: Adoo <Adoo@outlook.com>
Date: Wed, 4 Sep 2024 18:51:04 +0800
Subject: [PATCH] =?UTF-8?q?fix(macros):=20=F0=9F=90=9B=20variable=20parent?=
 =?UTF-8?q?=20with=20built-in=20fields=20need=20be=20mutable=20for=20its?=
 =?UTF-8?q?=20children?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 CHANGELOG.md              |  4 +++
 macros/src/declare_obj.rs | 67 ++++++++++++++++++++-------------------
 tests/rdl_macro_test.rs   | 12 +++++++
 3 files changed, 51 insertions(+), 32 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 571f0cf12..26a18f90a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,10 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
 
 ## [@Unreleased] - @ReleaseDate
 
+### Fixed
+
+- **macros**: Declaring the variable parent with built-in fields as immutable is incorrect if its child uses it as mutable. (#pr @M-Adoo)
+
 ## [0.4.0-alpha.7] - 2024-09-04
 
 ### Fixed
diff --git a/macros/src/declare_obj.rs b/macros/src/declare_obj.rs
index ecb9e8eb6..989d28a5f 100644
--- a/macros/src/declare_obj.rs
+++ b/macros/src/declare_obj.rs
@@ -3,7 +3,7 @@ use quote::{quote, quote_spanned, ToTokens};
 use syn::{
   punctuated::Punctuated,
   spanned::Spanned,
-  token::{Brace, Comma},
+  token::{Brace, Comma, Semi},
   Ident, Macro, Path,
 };
 
@@ -47,23 +47,37 @@ impl<'a> ToTokens for DeclareObj<'a> {
       self.gen_node_tokens(tokens);
     } else {
       Brace(*span).surround(tokens, |tokens| {
-        let mut node = quote! {};
-        self.gen_node_tokens(&mut node);
-        match &self.this.node_type {
-          ObjType::Type { span, .. } => {
-            // declare the host widget before  children. So that we can use variable if it
-            // moved in children. This is consistent with the user's declaration.
-            let name = Ident::new("_ribir_ಠ_ಠ", self.span);
-            quote_spanned! { *span => let _ribir_ಠ_ಠ = #node; }.to_tokens(tokens);
-            self.declare_children_and_compose_it(&name, tokens);
-          }
-          ObjType::Var(var) => {
-            if !this.fields.is_empty() {
-              quote_spanned! { var.span() => let #var = #node; }.to_tokens(tokens);
+        let name = match &this.node_type {
+          ObjType::Type { span, .. } => Ident::new("_ribir_ಠ_ಠ", *span),
+          ObjType::Var(var) => (*var).clone(),
+        };
+
+        // Declare the host widget before its children. This way, we can use variables
+        // if they are moved within the children, aligning with the user's declaration
+        // style.
+        if !matches!(this.node_type, ObjType::Var(_) if this.fields.is_empty()) {
+          if matches!(this.node_type, ObjType::Var(_) if !self.children.is_empty()) {
+            // If the object is a variable, after composing it with built-in objects, we
+            // should keep it mutable so that the children can utilize it.
+            quote! {
+              #[allow(unused_mut)]
+              let mut #name =
             }
-            self.declare_children_and_compose_it(var, tokens)
+            .to_tokens(tokens);
+          } else {
+            quote! { let #name = }.to_tokens(tokens);
           }
-        };
+          self.gen_node_tokens(tokens);
+          Semi(self.span).to_tokens(tokens);
+        }
+
+        let mut children = Vec::with_capacity(self.children.len());
+        for (i, c) in self.children.iter().enumerate() {
+          let child = Ident::new(&format!("_child_{i}_ಠ_ಠ"), c.span());
+          quote_spanned! { c.span() => let #child = #c; }.to_tokens(tokens);
+          children.push(child)
+        }
+        quote_spanned! { self.span => #name #(.with_child(#children))* }.to_tokens(tokens)
       })
     }
   }
@@ -86,17 +100,6 @@ impl<'a> DeclareObj<'a> {
     Ok(())
   }
 
-  fn declare_children_and_compose_it(&self, var: &Ident, tokens: &mut TokenStream) {
-    let mut children = vec![];
-    for (i, c) in self.children.iter().enumerate() {
-      let child = Ident::new(&format!("_child_{i}_ಠ_ಠ"), c.span());
-      quote_spanned! { c.span() => let #child = #c; }.to_tokens(tokens);
-      children.push(child)
-    }
-
-    quote_spanned! { self.span => #var #(.with_child(#children))* }.to_tokens(tokens)
-  }
-
   fn gen_node_tokens(&self, tokens: &mut TokenStream) {
     let ObjNode { node_type, fields } = &self.this;
     match node_type {
@@ -129,11 +132,11 @@ impl<'a> DeclareObj<'a> {
         } else {
           let fields = fields.iter();
           // move `var` last, so that we can use it in the fields.
-          quote_spanned! { var.span() => {
-            let mut _ಠ_ಠ = FatObj::new(());
-            #(_ಠ_ಠ = _ಠ_ಠ #fields;)*
-            _ಠ_ಠ.map(move |_| #var)
-          }}
+          quote_spanned! { var.span() =>
+            FatObj::new(())
+              #(#fields)*
+              .with_child(#var)
+          }
           .to_tokens(tokens);
         }
       }
diff --git a/tests/rdl_macro_test.rs b/tests/rdl_macro_test.rs
index 569430d1f..357b24daf 100644
--- a/tests/rdl_macro_test.rs
+++ b/tests/rdl_macro_test.rs
@@ -872,3 +872,15 @@ fn fix_direct_use_map_writer_with_builtin() {
       .map_writer(|w| PartData::from_ref_mut(&mut w.anchor));
   }
 }
+
+#[test]
+fn fix_use_var_in_children() {
+  let _w = fn_widget! {
+    let p = @MockBox { size: Size::zero() };
+    @ $p {
+      opacity: 1.,
+      // Use layout size query write of `p`
+      @MockBox { opacity: $p.opacity }
+    }
+  };
+}