Re: com php-src: Fix bug #68446 (bug with constant defaults and type hints): Zend/tests/class_constants_002.phpt Zend/zend_compile.c
Zend/zend_execute.c Zend/zend_vm_def.h Zend/zend_vm_execute.h

Wait. Master works just as expected with _no_ BC breaks. I just don't understand what's different in master than in 5.6, but somehow it works there just as expected. It's just 5.6 which needs special handling here. Bob > Am 25.11.2014 um 13:17 schrieb Dmitry Stogov <dmitry@zend.com>: > > Yes, we may make BC breaks in master, but they should be done on purpose and not just because > we can. > > I'll think how to fix it in PHP5.6, but in case I find a solution I'll need to revert > your changes in master anyway. > And I afraid, I can miss some peaces of your changes. > > Thanks. Dmitry. > > On Tue, Nov 25, 2014 at 3:08 PM, Bob Weinand <bobwei9@hotmail.com > <mailto:bobwei9@hotmail.com>> wrote: > Hey, > > Reverted in 5.6. > I now didn't revert master, especially as that gets a completely merge mess to revert. > Master is basically a separate re-implementation which seems to work without breaks. > That way also, when we merge up, we need to anyway throw away the PHP-5.6 branch changes there, > so the mess is unavoidable either way. > > Feel free to look at it, if you have a better idea :-) > > Thanks, > Bob > >> Am 25.11.2014 um 12:16 schrieb Dmitry Stogov <dmitry@zend.com >> <mailto:dmitry@zend.com>>: >> >> Hi Bob, >> >> Thanks. Please revert it for master as well. >> Once you find a good fix you may commit it into 5.6 and master again. >> Otherwise we will get a mess. >> >> If you like, I may try to look into the problem and try to fix it as well. >> >> Thanks. Dmitry. >> >> On Tue, Nov 25, 2014 at 2:03 PM, Bob Weinand <bobwei9@hotmail.com >> <mailto:bobwei9@hotmail.com>> wrote: >>> Am 24.11.2014 um 13:12 schrieb Dmitry Stogov <dmitry@zend.com >>> <mailto:dmitry@zend.com>>: >>> >>> On Mon, Nov 24, 2014 at 2:04 PM, Bob Weinand <bobwei9@hotmail.com >>> <mailto:bobwei9@hotmail.com>> wrote: >>> Hey Dmitry, >>> >>> My changes shouldn't break code which isn't already fragile. >>> class_constants_002.phpt is such an example. >>> >>> This is definitely a break >>> >>> php-5.5 -n -r 'function foo($x=Foo::Bar){var_dump($x);} foo(10);' >>> int(10) >>> >>> php-5.6 -n -r 'function foo($x=Foo::Bar){var_dump($x);} foo(10);' >>> >>> Fatal error: Class 'Foo' not found in Command line code on line 1 >>> >>> In some framework code it'll trigger Foo autoloading where it didn't do it >>> before. >> >> It's a behavior change, but it shouldn't break code. But I agree, that's bad >> for a micro release. >> >>> Though, I could restore old behavior and just evaluate in case a constant is passed >>> _and_ there is an array/callable typehint. Would that be fine? >>> In this case there wouldn't be any BC break (only already fatal-ing code working >>> again). >>> >>> This BC break is not the single problem in the patch, I actually noticed it because of >>> slight performance degradation. Also, could you explain this change: allow_null is defined as >>> zend_bool, but now it can be initialized with 1 or 2? >>> >>> @@ -1931,8 +1931,8 @@ void zend_do_receive_param(zend_uchar op, znode *varname, const >>> znode *initializ >>> if (class_type->u.constant.type == IS_ARRAY) { >>> cur_arg_info->type_hint = IS_ARRAY; >>> if (op == ZEND_RECV_INIT) { >>> - if (Z_TYPE(initialization->u.constant) == >>> IS_NULL || (Z_TYPE(initialization->u.constant) == IS_CONSTANT && >>> !strcasecmp(Z_STRVAL(initialization->u.constant), "NULL")) || >>> Z_TYPE(initialization->u.constant) == IS_CONSTANT_AST) { >>> - cur_arg_info->allow_null = 1; >>> + if (Z_TYPE(initialization->u.constant) == >>> IS_NULL || (Z_TYPE(initialization->u.constant) & IS_CONSTANT_TYPE_MASK) == IS_CONSTANT || >>> (Z_TYPE(initialization->u.constant) & IS_CONSTANT_TYPE_MASK) == IS_CONSTANT_AST) { >>> + cur_arg_info->allow_null = >>> (Z_TYPE(initialization->u.constant) != IS_NULL) + 1; >>> } else if >>> (Z_TYPE(initialization->u.constant) != IS_ARRAY) { >>> zend_error_noreturn(E_COMPILE_ERROR, >>> "Default value for parameters with array type hint can only be an array or NULL"); >>> } >>> >>> I would prefer, if you revert your patch, and then propose better solution (without BC >>> break). >>> you patch is not simple, and analyzing one fix on top of another is not going to be >>> simple. >> >> I somehow did something better when reimplementing for master, there >> class_constants_002.phpt is still the same and passes. I just don't understand _why_... I >> wonder now if we should leave that bug in 5.6 (and revert it there) or try to fix it there too? I >> will revert it today (because of 5.6.4rc1) if I don't get another reply from you today. Maybe >> you're able to quickly get what the difference between 5.6 and master is there? >> >> Bob >> >>> Thanks. Dmitry. >>> >>> >>> Thanks, >>> Bob >>> >>>> Am 24.11.2014 um 09:26 schrieb Dmitry Stogov <dmitry@zend.com >>>> <mailto:dmitry@zend.com>>: >>>> >>>> Hi Bob, >>>> >>>> Please, send such changes for review before committing. >>>> >>>> It is not a trivial change and it breaks existing behavior >>>> (Zend/tests/class_constants_002.phpt). >>>> Default values don't have to be evaluated, if actual parameter was sent. >>>> Your patch may affect existing code in unpredictable way (e.g. start fail because >>>> of error or trigger __autoload()). >>>> Please, revert it. Such changes can't be done in minor releases. >>>> Probably, the bug may be fixed in another way. >>>> >>>> Thanks. Dmitry. >>>> >>>> On Sun, Nov 23, 2014 at 11:10 PM, Bob Weinand <bwoebi@php.net >>>> <mailto:bwoebi@php.net>> wrote: >>>> Commit: 5ef138b0c7c4e9532e205f45c18a72aa1d279c24 >>>> Author: Bob Weinand <bobwei9@hotmail.com <§AàºBQG >>>> R,ˆ9[Pmailto:bobwei9@hotmail.com>> Sun, 23 Nov 2014 21:09:31 +0100 >>>> Parents: c8dd41554387e100a09811cad7f7032a291a79c2 >>>> Branches: PHP-5.6 >>>> >>>> Link: >>>> http://git.php.net/?p=php-src.git;a=commitdiff;h=5ef138b0c7c4e9532e205f45c18a72aa1d279c24 >>>> <http://git.php.net/?p=php-src.git;a=commitdiff;h=5ef138b0c7c4e9532e205f45c18a72aa1d279c24> >>>> >>>> Log: >>>> Fix bug #68446 (bug with constant defaults and type hints) >>>> >>>> Bugs: >>>> https://bugs.php.net/68446 >>>> <https://bugs.php.net/68446> >>>> >>>> Changed paths: >>>> M Zend/tests/class_constants_002.phpt >>>> M Zend/zend_compile.c >>>> M Zend/zend_execute.c >>>> M Zend/zend_vm_def.h >>>> M Zend/zend_vm_execute.h >>>> >>>> >>>> -- >>>> PHP CVS Mailing List (http://www.php.net/ >>>> <http://www.php.net/>) >>>> To unsubscribe, visit: http://www.php.net/unsub.php >>>> <http://www.php.net/unsub.php> >>>> >>> >>> >> >> > >