Avoid @prefer-ref arguments #16961
Avoid @prefer-ref arguments #16961
Conversation
| @@ -542,9 +542,9 @@ static zend_never_inline zend_ffi_cdata *zend_ffi_cdata_to_zval_slow_ret(void *p | |||
| } | |||
| /* }}} */ | |||
|
|
|||
| static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, void *ptr, zend_ffi_type *type, int read_type, zval *rv, zend_ffi_flags flags, bool is_ret, bool debug_union) /* {{{ */ | |||
| static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, void *ptr, zend_ffi_type *type, int convert, zval *rv, zend_ffi_flags flags, bool is_ret, bool debug_union) /* {{{ */ | |||
There was a problem hiding this comment.
Seems like this is now just used as a boolean flag?
| static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, void *ptr, zend_ffi_type *type, int convert, zval *rv, zend_ffi_flags flags, bool is_ret, bool debug_union) /* {{{ */ | |
| static zend_always_inline void zend_ffi_cdata_to_zval(zend_ffi_cdata *cdata, void *ptr, zend_ffi_type *type, bool convert, zval *rv, zend_ffi_flags flags, bool is_ret, bool debug_union) /* {{{ */ |
| || EX(opline)->opcode == ZEND_FETCH_DIM_FUNC_ARG) | ||
| && EX(call)->func | ||
| && EX(call)->func->type == ZEND_INTERNAL_FUNCTION | ||
| && EX(call)->func->internal_function.module == &ffi_module_entry) { |
There was a problem hiding this comment.
Just a note that this will unfortunately not work with frameless calls, since they do not create a stack frame.
There was a problem hiding this comment.
Right. may be you see a better (simpler) possible test.
Ideally, we might add some flag to opline->extended_value for FETCH_DIM/OBJ_R/FUNC_ARG at compile time.
This is possible for frame-less functions, but not possible for indirect calls. ($f = "FFI::type"; $f($x)).
| || EX(opline)->opcode == ZEND_FETCH_DIM_FUNC_ARG) | ||
| && EX(call)->func | ||
| && EX(call)->func->type == ZEND_INTERNAL_FUNCTION | ||
| && EX(call)->func->internal_function.module == &ffi_module_entry) { |
There was a problem hiding this comment.
There might be an issue (I didn't test it) when passing a value from an FFI value to one of the FFI functions that are currently not @prefer-ref. For example:
\FFI::memset($x, $y['z'], 42);
If $y is a CData, $y['z'] will return another CData object to represent the offset. This would throw a type error, at least with strict_types=1. Luckily, it doesn't look like there are many such cases. In theory, they could also be solved by accepting CData explicitly, and fetching the value internally.
There was a problem hiding this comment.
Right again. We should check that the result of FETCH_DIM/OBJ used as an operand of SEND or FRAMELESS_CALL and make a final decision taking into account the called function and parameter. The test becomes too complex...
There was a problem hiding this comment.
I had the same thought. This would be trivial for FETCH_DIM_R. It only uses flags in extended_value. FETCH_OBJ_R contains the cache slot offset in extended_value. However, it also stores ZEND_FETCH_REF (1), so we already remove it through a bitwise op when fetching the cache slot. We could either use 2, as the cache slot offset is a multiple of 4 on 32-bit, and 8 on 64-bit. Hence, there shouldn't be any performance overhead.
Reserving an engine flag for an optional extension does feel a bit unfortunate. That said, we likely could still use higher flags (1 << 31) when we need to.
No description provided.