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