rlerdorf commented 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.


----------------------------------------------------------------
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]


Reply via email to