fix(macro): reference mutability inside Option #515 (#615)

* fix(macro): reference mutability inside Option #515

* fix(macro): reference mutability inside Option #515

Author: kakserpom

crates/macros/src/function.rs

@@ -540,9 +540,11 @@ impl<'a> Args<'a> {
                                 .and_then(|ga| match ga {
                                     GenericArgument::Type(ty) => Some(match ty {
                                         Type::Reference(r) => {
+                                            // Only mark as_ref for mutable references
+                                            // (Option<&mut T>), not immutable ones (Option<&T>)
+                                            as_ref = r.mutability.is_some();
                                             let mut new_ref = r.clone();
                                             new_ref.mutability = None;
-                                            as_ref = true;
                                             Type::Reference(new_ref)
                                         }
                                         _ => ty.clone(),

src/types/array/mod.rs

@@ -5,7 +5,7 @@ use std::{convert::TryFrom, ffi::CString, fmt::Debug, ptr};
 
 use crate::{
     boxed::{ZBox, ZBoxable},
-    convert::{FromZval, IntoZval},
+    convert::{FromZval, FromZvalMut, IntoZval},
     error::Result,
     ffi::zend_ulong,
     ffi::{
@@ -711,3 +711,11 @@ impl<'a> FromZval<'a> for &'a ZendHashTable {
         zval.array()
     }
 }
+
+impl<'a> FromZvalMut<'a> for &'a mut ZendHashTable {
+    const TYPE: DataType = DataType::Array;
+
+    fn from_zval_mut(zval: &'a mut Zval) -> Option<Self> {
+        zval.array_mut()
+    }
+}

src/types/zval.rs

@@ -14,8 +14,8 @@ use crate::{
     error::{Error, Result},
     ffi::{
         _zval_struct__bindgen_ty_1, _zval_struct__bindgen_ty_2, ext_php_rs_zend_string_release,
-        zend_is_callable, zend_is_identical, zend_is_iterable, zend_resource, zend_value, zval,
-        zval_ptr_dtor,
+        zend_array_dup, zend_is_callable, zend_is_identical, zend_is_iterable, zend_resource,
+        zend_value, zval, zval_ptr_dtor,
     },
     flags::DataType,
     flags::ZvalTypeFlags,
@@ -224,9 +224,30 @@ impl Zval {
 
     /// Returns a mutable reference to the underlying zval hashtable if the zval
     /// contains an array.
+    ///
+    /// # Array Separation
+    ///
+    /// PHP arrays use copy-on-write (COW) semantics. Before returning a mutable
+    /// reference, this method checks if the array is shared (refcount > 1) and
+    /// if so, creates a private copy. This is equivalent to PHP's
+    /// `SEPARATE_ARRAY()` macro and prevents the "Assertion failed:
+    /// `zend_gc_refcount` == 1" error that occurs when modifying shared arrays.
     pub fn array_mut(&mut self) -> Option<&mut ZendHashTable> {
         if self.is_array() {
-            unsafe { self.value.arr.as_mut() }
+            unsafe {
+                let arr = self.value.arr;
+                // Check if the array is shared (refcount > 1)
+                // If so, we need to separate it (copy-on-write)
+                if (*arr).gc.refcount > 1 {
+                    // Decrement the refcount of the original array
+                    (*arr).gc.refcount -= 1;
+                    // Duplicate the array to get our own private copy
+                    let new_arr = zend_array_dup(arr);
+                    // Update the zval to point to the new array
+                    self.value.arr = new_arr;
+                }
+                self.value.arr.as_mut()
+            }
         } else {
             None
         }

tests/src/integration/array/array.php

@@ -79,3 +79,18 @@
 
 assert(array_key_exists('00', $leading_zeros), '"00" should stay as string key');
 assert($leading_zeros['00'] === 'zerozero', 'Value at key "00" should be "zerozero"');
+
+// Test Option<&ZendHashTable> with literal array (issue #515)
+// This should work without "could not be passed by reference" error
+assert(test_optional_array_ref([1, 2, 3]) === 3, 'Option<&ZendHashTable> should accept literal array');
+assert(test_optional_array_ref(null) === -1, 'Option<&ZendHashTable> should accept null');
+$arr = ['a', 'b', 'c', 'd'];
+assert(test_optional_array_ref($arr) === 4, 'Option<&ZendHashTable> should accept variable array');
+
+// Test Option<&mut ZendHashTable> (anti-regression for issue #515)
+$mut_arr = ['x', 'y'];
+assert(test_optional_array_mut_ref($mut_arr) === 3, 'Option<&mut ZendHashTable> should accept variable array and add element');
+assert(array_key_exists('added_by_rust', $mut_arr), 'Rust should have added a key to the array');
+assert($mut_arr['added_by_rust'] === 'value', 'Added value should be correct');
+$null_arr = null;
+assert(test_optional_array_mut_ref($null_arr) === -1, 'Option<&mut ZendHashTable> should accept null');

tests/src/integration/array/mod.rs

@@ -5,7 +5,7 @@ use ext_php_rs::{
     ffi::HashTable,
     php_function,
     prelude::ModuleBuilder,
-    types::{ArrayKey, Zval},
+    types::{ArrayKey, ZendHashTable, Zval},
     wrap_function,
 };
 
@@ -41,13 +41,34 @@ pub fn test_array_keys() -> Zval {
     ht.into_zval(false).unwrap()
 }
 
+/// Test that `Option<&ZendHashTable>` can accept literal arrays (issue #515)
+#[php_function]
+pub fn test_optional_array_ref(arr: Option<&ZendHashTable>) -> i64 {
+    arr.map_or(-1, |ht| i64::try_from(ht.len()).unwrap_or(i64::MAX))
+}
+
+/// Test that `Option<&mut ZendHashTable>` works correctly (anti-regression for issue #515)
+#[php_function]
+pub fn test_optional_array_mut_ref(arr: Option<&mut ZendHashTable>) -> i64 {
+    match arr {
+        Some(ht) => {
+            // Add an element to verify mutation works
+            ht.insert("added_by_rust", "value").ok();
+            i64::try_from(ht.len()).unwrap_or(i64::MAX)
+        }
+        None => -1,
+    }
+}
+
 pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
     builder
         .function(wrap_function!(test_array))
         .function(wrap_function!(test_array_assoc))
         .function(wrap_function!(test_array_assoc_array_keys))
         .function(wrap_function!(test_btree_map))
         .function(wrap_function!(test_array_keys))
+        .function(wrap_function!(test_optional_array_ref))
+        .function(wrap_function!(test_optional_array_mut_ref))
 }
 
 #[cfg(test)]