rlerdorf edited a comment on pull request #2288:
URL: https://github.com/apache/thrift/pull/2288#issuecomment-772844645
This looks good, @tylerchr. The only thing you missed is that the
`$buffer_size` argument is optional on the two read_binary functions. So in
your stub file, provide the default value from the source which is 8192. Like
this:
```php
function thrift_protocol_read_binary(object $protocol, string $obj_typename,
bool $strict_read, int $buffer_size=8192): object {}
function thrift_protocol_read_binary_after_message_begin(object $protocol,
string $obj_typename, bool $strict_read, int $buffer_size=8192): object {}
```
And regenerate the arginfo.h file, of course.
You can also add `/** @generate-function-entries */` to your stub file to
get it to generate the function entries and pull those out of the `.cpp` file,
but it isn't necessary.
And another optional thing. To address your initial issue of the arginfo
failing with PHP 7.2 and earlier, you can macro your way around that issue
using:
```
#ifndef ZEND_ARG_INFO_WITH_DEFAULT_VALUE
#define ZEND_ARG_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, default_value) \
ZEND_ARG_INFO(pass_by_ref, name)
#endif
#if PHP_VERSION_ID < 70200
#undef ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX
#define ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(name, return_reference,
required_num_args, class_name, allow_null) \
static const zend_internal_arg_info name[] = { \
{ (const char*)(zend_uintptr_t)(required_num_args), (
#class_name ), 0, return_reference, allow_null, 0 },
#endif
#ifndef ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX
# define ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(name, return_reference,
required_num_args, class_name, allow_null) \
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(name, return_reference,
required_num_args, class_name, allow_null)
#endif
#ifndef ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX
# define ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(name, return_reference,
num_args, type) \
ZEND_BEGIN_ARG_INFO_EX(name, 0, return_reference, num_args)
#endif
#ifndef ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX
# define ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(name, return_reference,
required_num_args, class_name, type) \
ZEND_BEGIN_ARG_INFO_EX(name, 0, return_reference, required_num_args)
#endif
#ifndef ZEND_ARG_TYPE_MASK
# define ZEND_ARG_TYPE_MASK(pass_by_ref, name, type_mask, default_value) \
ZEND_ARG_TYPE_INFO(pass_by_ref, name, 0, 0)
#endif
#ifndef ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE
# define ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(pass_by_ref, name, type_hint,
allow_null, default_value) \
ZEND_ARG_TYPE_INFO(pass_by_ref, name, type_hint, allow_null)
#endif
```
But again, not required.
I reworked your patch slightly. This compiles cleanly with arginfo back to
PHP 7.0 -
https://github.com/rlerdorf/thrift/commit/a95df2539d9434dd5ac86a832e1c3cfcfa4c6097
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
