Re: Re: PHP 5.6.0beta2 Released for Testing!

From: Date: Fri, 02 May 2014 22:32:16 +0000
Subject: Re: Re: PHP 5.6.0beta2 Released for Testing!
References: 1 2 3 4 5 6 7 8  Groups: php.internals 
Request: Send a blank email to internals+get-73867@lists.php.net to get a copy of this message
Am 2.5.2014 um 23:27 schrieb Pierre Joye <pierre.php@gmail.com>: > hi Bob, > > On Fri, May 2, 2014 at 6:38 PM, Bob Weinand <bobwei9@hotmail.com> wrote: >> >> Am 2.5.2014 um 18:11 schrieb Ferenc Kovacs <tyra3l@gmail.com>: >> >>> On Fri, May 2, 2014 at 5:52 PM, Hannes Magnusson <bjori@php.net> wrote: >>> >>>> On Fri, May 2, 2014 at 6:17 AM, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote: >>>>> Ferenc Kovacs in php.qa (Fri, 2 May 2014 12:42:55 +0200): >>>>>>> ext\apcu\apc_bin.c(238) : error C2065: 'IS_CONSTANT_ARRAY' : >>>>>>> undeclared >>>>>>> identifier >>>>>> >>>>>> Afair this was mentioned in UPGRADING.INTERNALS, if I remember wrong >>>> please >>>>>> open a bugreport. >>>>> >>>>> Might be that it is mentioned there (did not check yet), but to an >>>>> innocent bystander it looks like a BC break between beta1 and beta2. >>>>> >>>> >>>> >>>> >>>> l. Removal of IS_CONSTANT_ARRAY and IS_CONSTANT_INDEX hack >>>> >>>> These two #defines disappeared. Instead we have now IS_CONSTANT_AST which >>>> covers also the functionality IS_CONSTANT_ARRAY bid and furthermore the >>>> hack for marking zvals as constant index with IS_CONSTANT_INDEX is now >>>> superfluous and so removed. >>>> Please note that IS_CONSTANT_AST now has the same value than >>>> IS_CONSTANT_ARRAY had. > > As I do not mind API changes between two x.y versions (it is allowed), > I wonder why removing it is a good thing. Given that IS_CONSTANT_AST > and IS_CONSTANT_ARRAY have the same values, the behaviors are the > same. Keeping the old and document it as alias.Or am I missing > something? It isn't the same. IS_CONSTANT_ARRAY is using zvalue_value.ht and IS_CONSTANT_AST is using zvalue_value.ast. If I'd made them an alias, that'd be totally confusing. It's just IS_CONSTANT_ARRAY which could disappear as it isn't needed anymore and IS_CONSTANT_AST now used the free gap there. It really should break the extensions there just as a hint to the maintainer that they need an update. Also if these extensions all needed to handle IS_CONSTANT_ARRAY, they probably will need a specific handling for IS_CONSTANT_AST, too. If I hadn't removed the IS_CONSTANT_ARRAY, people might only notice the bug (which appears due to inexistent IS_CONSTANT_AST handling) at PHP run-time, maybe only after 5.6 release. This removal sets a clear signal that these extensions need some update. It's at the same time the easiest to debug for maintainers when they get an error when compiling the extension... >> It maybe does the same thing, but it needs other handling. >> (IS_CONSTANT_AST in reality covers also many other cases, not >> just arrays). >> >> If I hadn't renamed the constant, it might eventually still compile fine, >> but the program would get crashes you don't notice when not testing >> a very specific subset of the language in combination with the AST. >> (In reality the AST is more an addition and now when fixing bug 66015 >> much later, IS_CONSTANT_AST became superfluous and so was removed). >> >> This at least ensures that you notice the problem. And prevents other >> people complaining about a bug you maybe don't find so easily. >> >> I just did what seemed the most sensible thing to me. > > It is, there is only little gain to remove it just to remove it. > > Either way I am fine with the changes, but other may not. > > ps: there is no need to get such bad wording to discuss technical > issues. There are people here actually doing the job, and doing it > great. A bit of respect would be more than welcome. Bob is one of > them. > > -- > Pierre > > @pierrejoye | http://www.libgd.org Thank you for the kind words :-) Bob

« previous php.internals (#73867) next »