Skip to content

Commit a66b8a6

Browse files
committed
fix(macro): reference mutability inside Option #515
1 parent 5449502 commit a66b8a6

File tree

4 files changed

+48
-3
lines changed

4 files changed

+48
-3
lines changed

crates/macros/src/function.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,9 +540,11 @@ impl<'a> Args<'a> {
540540
.and_then(|ga| match ga {
541541
GenericArgument::Type(ty) => Some(match ty {
542542
Type::Reference(r) => {
543+
// Only mark as_ref for mutable references
544+
// (Option<&mut T>), not immutable ones (Option<&T>)
545+
as_ref = r.mutability.is_some();
543546
let mut new_ref = r.clone();
544547
new_ref.mutability = None;
545-
as_ref = true;
546548
Type::Reference(new_ref)
547549
}
548550
_ => ty.clone(),

src/types/array/mod.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{convert::TryFrom, ffi::CString, fmt::Debug, ptr};
55

66
use crate::{
77
boxed::{ZBox, ZBoxable},
8-
convert::{FromZval, IntoZval},
8+
convert::{FromZval, FromZvalMut, IntoZval},
99
error::Result,
1010
ffi::zend_ulong,
1111
ffi::{
@@ -711,3 +711,11 @@ impl<'a> FromZval<'a> for &'a ZendHashTable {
711711
zval.array()
712712
}
713713
}
714+
715+
impl<'a> FromZvalMut<'a> for &'a mut ZendHashTable {
716+
const TYPE: DataType = DataType::Array;
717+
718+
fn from_zval_mut(zval: &'a mut Zval) -> Option<Self> {
719+
zval.array_mut()
720+
}
721+
}

tests/src/integration/array/array.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,17 @@
7979

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

tests/src/integration/array/mod.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use ext_php_rs::{
55
ffi::HashTable,
66
php_function,
77
prelude::ModuleBuilder,
8-
types::{ArrayKey, Zval},
8+
types::{ArrayKey, ZendHashTable, Zval},
99
wrap_function,
1010
};
1111

@@ -41,13 +41,34 @@ pub fn test_array_keys() -> Zval {
4141
ht.into_zval(false).unwrap()
4242
}
4343

44+
/// Test that `Option<&ZendHashTable>` can accept literal arrays (issue #515)
45+
#[php_function]
46+
pub fn test_optional_array_ref(arr: Option<&ZendHashTable>) -> i64 {
47+
arr.map_or(-1, |ht| i64::try_from(ht.len()).unwrap_or(i64::MAX))
48+
}
49+
50+
/// Test that `Option<&mut ZendHashTable>` works correctly (anti-regression for issue #515)
51+
#[php_function]
52+
pub fn test_optional_array_mut_ref(arr: Option<&mut ZendHashTable>) -> i64 {
53+
match arr {
54+
Some(ht) => {
55+
// Add an element to verify mutation works
56+
ht.insert("added_by_rust", "value").ok();
57+
i64::try_from(ht.len()).unwrap_or(i64::MAX)
58+
}
59+
None => -1,
60+
}
61+
}
62+
4463
pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder {
4564
builder
4665
.function(wrap_function!(test_array))
4766
.function(wrap_function!(test_array_assoc))
4867
.function(wrap_function!(test_array_assoc_array_keys))
4968
.function(wrap_function!(test_btree_map))
5069
.function(wrap_function!(test_array_keys))
70+
.function(wrap_function!(test_optional_array_ref))
71+
.function(wrap_function!(test_optional_array_mut_ref))
5172
}
5273

5374
#[cfg(test)]

0 commit comments

Comments
 (0)