Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/macros/src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
10 changes: 9 additions & 1 deletion src/types/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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()
}
}
27 changes: 24 additions & 3 deletions src/types/zval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions tests/src/integration/array/array.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
23 changes: 22 additions & 1 deletion tests/src/integration/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ext_php_rs::{
ffi::HashTable,
php_function,
prelude::ModuleBuilder,
types::{ArrayKey, Zval},
types::{ArrayKey, ZendHashTable, Zval},
wrap_function,
};

Expand Down Expand Up @@ -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)]
Expand Down