[PHP-QA] Re: PHP 5.6.0beta2 Released for Testing! View as Markdown

Ferenc Kovacs in php.internals (Fri, 2 May 2014 18:28:12 +0200):

from a quick search search on lxr.php.net the following extensions are
using IS_CONSTANT_ARRAY or IS_CONSTANT_INDEX

• PHK
• Parse_Tree
• automap
• bcompiler
• hidef
• AOP
• yaf
• APC
• ZendOpcache
• APCu
• vld
• WinCache

and probably a couple of other exts which isn't on lxr.

You can add xcache and qb to the list. QB is a Pecl extension nowadays
and xcache probably never will be.

Jan

Am 2.5.2014 um 18:28 schrieb Ferenc Kovacs tyra3l@gmail.com:

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.

Could someone be.. like just a tiny littlebit more professional then
to shit over other people work in the upgrading notes?
This is not the place for to show off how much better you are then
everyone else.

And why does it have to be renamed anyway? It has the same value. Does
the same thing. It just got renamed to piss over people?

-Hannes

--
PHP Quality Assurance Mailing List http://www.php.net/

moving the discussion to internals@, CCing Bob, as he is the author of
the change.
To my understanding this change was made to fix
https://bugs.php.net/bug.php?id=66015
I was aware that it is a BC break, but I didn't realized that it will
affect other extensions before the first report from the wincache dev
arrived.
Not sure if removing this #defines is really neccessary to fix the bug
mentioned above.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

from a quick search search on lxr.php.net the following extensions are
using IS_CONSTANT_ARRAY or IS_CONSTANT_INDEX

• PHK
• Parse_Tree
• automap
• bcompiler
• hidef
• AOP
• yaf
• APC
• ZendOpcache
• APCu
• vld
• WinCache

and probably a couple of other exts which isn't on lxr.
whats even more interesting is that ext/opcache still referencing both of
those, so it isn't even cleaned up from php-src.
what is the current workflow for opcache these days? I know that they have
their own repo, and they are merging from there to php-src.git and that
they are also maintain compatibility with lower versions for installing
opcache via pecl, so maybe it is normal to have those references?

personally I think that compatibility with already existing extensions is
pretty important, because that is a major reason which holds back people
from upgrading to new php versions early.
so internal API breaks like that should happen early in the development
process, have a clear communication and should be always for a good reason.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

At the time when doing this change, I had fixed all the bundled extensions.

So, e.g. opcache shouldn't be a problem.

Actually, I made the commit beginning of April, I think that are nearly
three months time to fix the extensions. I doubt that isn't enough…

Bob

Bob

Ferenc Kovacs in php.internals (Fri, 2 May 2014 18:28:12 +0200):

personally I think that compatibility with already existing extensions is
pretty important, because that is a major reason which holds back people
from upgrading to new php versions early.

The hammer on the nail. I will not promote beta2 on Apachelounge because
of these BC breaks.

Jan

Am 2.5.2014 um 18:11 schrieb Ferenc Kovacs tyra3l@gmail.com:

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.

Could someone be.. like just a tiny littlebit more professional then
to shit over other people work in the upgrading notes?
This is not the place for to show off how much better you are then
everyone else.

And why does it have to be renamed anyway? It has the same value. Does
the same thing. It just got renamed to piss over people?

-Hannes

--
PHP Quality Assurance Mailing List http://www.php.net/

moving the discussion to internals@, CCing Bob, as he is the author of the
change.
To my understanding this change was made to fix
https://bugs.php.net/bug.php?id=66015
I was aware that it is a BC break, but I didn't realized that it will
affect other extensions before the first report from the wincache dev
arrived.
Not sure if removing this #defines is really neccessary to fix the bug
mentioned above.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

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).

Fair enough.
Can you please fix the language in the upgrading file though?
This is very disrespectful and I shouldn't have to read internal
argument you have with someone in upgrade documentations.

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 think the value needs to be modified - for the same reason as you renamed it.

Who knows, maybe someone has #ifndef..#define is_constant_array .. as
it was a new addition at some point.

Now that code will kick in again and cause you whatever subtle issues
you are indicating as the reason for the rename.

-Hannes

hi Bob,

Am 2.5.2014 um 18:11 schrieb Ferenc Kovacs tyra3l@gmail.com:

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