FAQ
The PHP development team would like to announce the immediate
availability of PHP 5.3.8. This release fixes two issues introduced in
the PHP 5.3.7 release:

* Fixed bug #55439 (crypt() returns only the salt for MD5)
* Reverted a change in timeout handling restoring PHP 5.3.6
behavior, which caused mysqlnd SSL connections to hang (Bug
#55283).

All PHP users should note that the PHP 5.2 series is NOT supported
anymore. All users are strongly encouraged to upgrade to PHP 5.3.8.

For source downloads please visit our downloads page at
<http://php.net/downloads.php>, Windows binaries can be found on
<http://windows.php.net/download/>.

Johannes Schlüter
PHP 5.3 Release Master

Search Discussions

  • Alan at Aug 24, 2011 at 1:57 am
    It might have been better to have waited for the is_a() fix to get
    sorted out.. - It's a very annoying BC break (changes the documented
    behaviour), and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..

    Regards
    Alan
    On Wednesday, August 24, 2011 12:08 AM, Johannes Schlüter wrote:
    The PHP development team would like to announce the immediate
    availability of PHP 5.3.8. This release fixes two issues introduced in
    the PHP 5.3.7 release:

    * Fixed bug #55439 (crypt() returns only the salt for MD5)
    * Reverted a change in timeout handling restoring PHP 5.3.6
    behavior, which caused mysqlnd SSL connections to hang (Bug
    #55283).

    All PHP users should note that the PHP 5.2 series is NOT supported
    anymore. All users are strongly encouraged to upgrade to PHP 5.3.8.

    For source downloads please visit our downloads page at
    <http://php.net/downloads.php>, Windows binaries can be found on
    <http://windows.php.net/download/>.

    Johannes Schlüter
    PHP 5.3 Release Master

  • Ferenc Kovacs at Aug 24, 2011 at 7:44 am

    On Wed, Aug 24, 2011 at 3:57 AM, alan@akbkhome.com wrote:
    It might have been better to have waited for the is_a() fix to get sorted
    out.. - It's a very annoying BC break (changes the documented behaviour),
    and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:

    as Stas pointed out:
    "Not a bug. $var is interpreted as a class name. To know if one class
    extends another, the class in question (first one) should be loaded.
    There's no need to load the second one since if it's unknown nothing
    can be instance of it and existing classes can not extend it."
    if you used this previously

    from Dmitry:
    "Before the patch, is_a() didn't accept string as the first argument
    at all, so it always returned "false" and never triggered
    __autoload(). The proposed patch didn't revert to original behavior.
    It just disables autoloading and may lead to wrong result.

    class a {}
    class b extends a {}
    var_dump(is_a("b", "a")); // it was false before 5.3.7, now it's true

    I would say that the old behaviour was wrong, especially because
    "instanceof" and is_subclass_of() already implemented support for
    string arguments."

    so your example was bogus, as passing a non-object as a first
    parameter wasn't supported (see http://php.net/is_a) so your code
    example depends on an undefined behavior and results in a bogus result
    (is_a() alwas returned false if you passed a non-object)

    so in a way it is really a BC, but I think that this change is really a bugfix.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Derick Rethans at Aug 24, 2011 at 9:36 am

    On Wed, 24 Aug 2011, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 3:57 AM, alan@akbkhome.com wrote:
    It might have been better to have waited for the is_a() fix to get sorted
    out.. - It's a very annoying BC break (changes the documented behaviour),
    and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:
    It is also a BC break! Yes, it should be fixed, but only in 5.4. This
    kind of little changes is what makes people look at PHP as a joke!

    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug
  • Ferenc Kovacs at Aug 24, 2011 at 9:50 am

    On Wed, Aug 24, 2011 at 11:36 AM, Derick Rethans wrote:
    On Wed, 24 Aug 2011, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 3:57 AM, alan@akbkhome.com wrote:
    It might have been better to have waited for the is_a() fix to get sorted
    out.. - It's a very annoying BC break (changes the documented behaviour),
    and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:
    It is also a BC break! Yes, it should be fixed, but only in 5.4. This
    kind of little changes is what makes people look at PHP as a joke!
    agree with keeping the BC as much as possible, but from the responses
    from the others (Dmitry, Stas, Zeev) and the nature of the change
    (changing the undefined behavior of the function) and the fact that it
    is already out there (and changing this back and releasing a new
    version now or later would be even more wrong) my guess is that we
    will keep the new behavior.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Pierre Joye at Aug 24, 2011 at 10:00 am
    agreed.

    On Wed, Aug 24, 2011 at 11:36 AM, Derick Rethans wrote:
    On Wed, 24 Aug 2011, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 3:57 AM, alan@akbkhome.com wrote:
    It might have been better to have waited for the is_a() fix to get sorted
    out.. - It's a very annoying BC break (changes the documented behaviour),
    and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:
    It is also a BC break! Yes, it should be fixed, but only in 5.4. This
    kind of little changes is what makes people look at PHP as a joke!

    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php


    --
    Pierre

    @pierrejoye | http://blog.thepimp.net | http://www.libgd.org
  • Ferenc Kovacs at Aug 24, 2011 at 10:24 am

    On Wed, Aug 24, 2011 at 12:00 PM, Pierre Joye wrote:
    agreed.
    about this specific change:
    the real problem is that nobody spotted this change until we rolled
    out the stable release, after that we can only chose from bad options.
    we could have spotted this via two ways:
    - those who participated in fixing
    https://bugs.php.net/bug.php?id=53727 could have spotted this
    - our tests should have start failing after the change

    AFAIK non of that happened, and fixing the first is hard, so I would
    propose fixing the second (I know, it is already agreed and worked
    toward)

    another problem is that the current(and before the change also) proto
    was not forced by the function, so if is_a() would raise warnings when
    non-object is passed to the first argument, this BC break would have
    much less impact.

    so I got the idea that it would be usefull, if somebody could write a
    script which fetches the proto for the functions/methods and does some
    fuzzing which checks if the protos are enforced.
    after that, we would have a list, and probably we could fix those
    (changing the proto and the docs to the real behaviour, or vica
    versa), and we could see whether the fix would impy BC and for those
    cases we could just keep it as is for the stable branch and create
    tests which would guarantee that changing the current behavior in the
    stable branch wouldn't go unnoticed.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Pierre Joye at Aug 24, 2011 at 10:37 am
    the problem is to change existing tests to match a "possible fix",
    that defeats the whole purpose of testing possible BC breaks using our
    test suites.

    We should definitively run x.y.0 tests during the whole x.y.0 life,
    and if something has to be changed, then it should be 1st be
    discussed, or RFC for big changes. And then it should indeed be
    documented as well, in the UPGRADING guide (should be done anyway for
    x.y+1 versions too).
    On Wed, Aug 24, 2011 at 12:24 PM, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 12:00 PM, Pierre Joye wrote:
    agreed.
    about this specific change:
    the real problem is that nobody spotted this change until we rolled
    out the stable release, after that we can only chose from bad options.
    we could have spotted this via two ways:
    - those who participated in fixing
    https://bugs.php.net/bug.php?id=53727 could have spotted this
    - our tests should have start failing after the change

    AFAIK non of that happened, and fixing the first is hard, so I would
    propose fixing the second (I know, it is already agreed and worked
    toward)

    another problem is that the current(and before the change also) proto
    was not forced by the function, so if is_a() would raise warnings when
    non-object is passed to the first argument, this BC break would have
    much less impact.

    so I got the idea that it would be usefull, if somebody could write a
    script which fetches the proto for the functions/methods and does some
    fuzzing which checks if the protos are enforced.
    after that, we would have a list, and probably we could fix those
    (changing the proto and the docs to the real behaviour, or vica
    versa), and we could see whether the fix would impy BC and for those
    cases we could just keep it as is for the stable branch and create
    tests which would guarantee that changing the current behavior in the
    stable branch wouldn't go unnoticed.

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


    --
    Pierre

    @pierrejoye | http://blog.thepimp.net | http://www.libgd.org
  • Johannes Schlüter at Aug 24, 2011 at 10:42 am

    On Wed, 2011-08-24 at 12:24 +0200, Ferenc Kovacs wrote:
    we could have spotted this via two ways:
    - those who participated in fixing
    https://bugs.php.net/bug.php?id=53727 could have spotted this
    - our tests should have start failing after the change
    Third option:
    - RC testers might have spotted and reported it.

    I have the impression that very few people actually test these. When
    creating an RC we inform the "primary testers" as well as qa and
    internals list members. From there I get one or two responses in
    general.

    When I approach PHP users I often get answers like installing PHP
    without breaking their setup would be complicated (which is not the case
    but maybe needs education?) and they won't have time. I try to use a
    hypothetical case, like we have here in reality, to explain them why it
    is beneficial for their business if it is detected early as then we can
    fix it, fixing something after a release is hard. We also can try to
    improve our tests but we will never be able to test each and every way
    PHP is being used out there in the wild.


    So how can we motivate people to test new versions during RC not the day
    after it is being released?


    We don't push them out as news on the php.net frontpage and we don't
    send it out to the announce list for reasons like not confusing users.
    Should we change that? Other ideas?

    johannes
  • Dirk Haun at Aug 24, 2011 at 11:04 am

    Quoting Johannes Schlüter <johannes@schlueters.de>:

    I have the impression that very few people actually test these. When
    creating an RC we inform the "primary testers" as well as qa and
    internals list members. From there I get one or two responses in
    general.
    What happens to the reports that "make test" sends back to php.net?
    There are always a few tests failing (I don't think I ever installed a
    PHP version - final or not - that didn't have at least one or two
    failing tests).

    Where do those reports end up and is anyone actually looking at them?

    bye, Dirk
  • Ferenc Kovacs at Aug 24, 2011 at 11:16 am

    On Wed, Aug 24, 2011 at 1:04 PM, Dirk Haun wrote:
    Quoting Johannes Schlüter <johannes@schlueters.de>:
    I have the impression that very few people actually test these. When
    creating an RC we inform the "primary testers" as well as qa and
    internals list members. From there I get one or two responses in
    general.
    What happens to the reports that "make test" sends back to php.net? There
    are always a few tests failing (I don't think I ever installed a PHP version
    - final or not - that didn't have at least one or two failing tests).

    Where do those reports end up and is anyone actually looking at them?
    they are sent to the qa reports mailing list: http://news.php.net/php.qa.reports
    the aggregated report summaries are available at
    http://qa.php.net/reports/ thanks to Oliver Doucet who make this
    happen.

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Ryan McCue at Aug 24, 2011 at 11:59 am

    Ferenc Kovacs wrote:
    the aggregated report summaries are available at
    http://qa.php.net/reports/ thanks to Oliver Doucet who make this
    happen.
    Which, incidentally, doesn't appear to be working at this point in time.
  • Ferenc Kovacs at Aug 24, 2011 at 12:04 pm

    On Wed, Aug 24, 2011 at 1:59 PM, Ryan McCue wrote:
    Ferenc Kovacs wrote:
    the aggregated report summaries are available at
    http://qa.php.net/reports/ thanks to Oliver Doucet who make this
    happen.
    Which, incidentally, doesn't appear to be working at this point in time.
    it was working when I sent the mailt.
    it would be a weird coincidence...

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Dirk Haun at Aug 24, 2011 at 12:16 pm

    Quoting Ferenc Kovacs <tyra3l@gmail.com>:
    On Wed, Aug 24, 2011 at 1:59 PM, Ryan McCue wrote:
    Ferenc Kovacs wrote:
    the aggregated report summaries are available at
    http://qa.php.net/reports/ thanks to Oliver Doucet who make this
    happen.
    Which, incidentally, doesn't appear to be working at this point in time.
    it was working when I sent the mailt.
    it would be a weird coincidence...
    Works for me.

    However, it's still not clear (to me) what's happening to these
    reports once they're up there. Is anyone looking at them?

    bye, Dirk
  • Ferenc Kovacs at Aug 24, 2011 at 12:33 pm

    On Wed, Aug 24, 2011 at 2:15 PM, Dirk Haun wrote:
    Quoting Ferenc Kovacs <tyra3l@gmail.com>:
    On Wed, Aug 24, 2011 at 1:59 PM, Ryan McCue wrote:

    Ferenc Kovacs wrote:
    the aggregated report summaries are available at
    http://qa.php.net/reports/ thanks to Oliver Doucet who make this
    happen.
    Which, incidentally, doesn't appear to be working at this point in time.
    it was working when I sent the mailt.
    it would be a weird coincidence...
    Works for me.

    However, it's still not clear (to me) what's happening to these reports once
    they're up there. Is anyone looking at them?
    it should be.
    AFAIK the core devs mostly check our own build reports
    (http://gcov.php.net/ and Pierres buildbot results at
    http://windows.php.net/downloads/snaps/php-trunk ) only.
    the QA team isn't really active as a group, hovewer half of the
    members are active as developers (see the http://php.net/credits.php
    Quality Assurance Team)

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Ferenc Kovacs at Aug 24, 2011 at 11:08 am

    2011/8/24 Johannes Schlüter <johannes@schlueters.de>:
    On Wed, 2011-08-24 at 12:24 +0200, Ferenc Kovacs wrote:
    we could have spotted this via two ways:
    - those who participated in fixing
    https://bugs.php.net/bug.php?id=53727 could have spotted this
    - our tests should have start failing after the change
    Third option:
    - RC testers might have spotted and reported it.

    I have the impression that very few people actually test these. When
    creating an RC we inform the "primary testers" as well as qa and
    internals list members. From there I get one or two responses in
    general.

    When I approach PHP users I often get answers like installing PHP
    without breaking their setup would be complicated (which is not the case
    but maybe needs education?) and they won't have time. I try to use a
    hypothetical case, like we have here in reality, to explain them why it
    is beneficial for their business if it is detected early as then we can
    fix it, fixing something after a release is hard. We also can try to
    improve our tests but we will never be able to test each and every way
    PHP is being used out there in the wild.


    So how can we motivate people to test new versions during RC not the day
    after it is being released?


    We don't push them out as news on the php.net frontpage and we don't
    send it out to the announce list for reasons like not confusing users.
    Should we change that? Other ideas?

    johannes
    agree, should have mentioned that.
    I think that currently testing the RCs have a very high barrier.
    usually they are going unnoticed for most people and compiling your
    own version (with all the extensions that you need) can be really
    cumbersome.
    - we need to get out the word to the masses (the current php.net site
    simply lacks this, maybe the http://prototype.php.net/ will be better
    in this regard), for which we also need to lower the barriers to
    entry:
    - better documentation about how to build your own php version would
    be a must, maybe phpfarm can be also useful for this
    - we should cooperate with the major php projects out there to run
    their testsuites against the new releases or maybe even trunk, if I
    remember correctly somebody mentioned that we already do this for some
    projects (maybe Pierre mentioned this). this would be an easy way to
    boost our test coverage and make the BC breaks more obvious.
    - having pre-packaged versions of php available would also help,
    testing out the latest mysql versions are much more easy for example,
    as I can just grab the Linux - Generic archive, extract it, and voila,
    I can test it.
    - projects like http://apt.damz.org/ also help

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Alexey Shein at Aug 24, 2011 at 11:28 am

    2011/8/24 Ferenc Kovacs <tyra3l@gmail.com>:
    2011/8/24 Johannes Schlüter <johannes@schlueters.de>:
    On Wed, 2011-08-24 at 12:24 +0200, Ferenc Kovacs wrote:
    we could have spotted this via two ways:
    - those who participated in fixing
    https://bugs.php.net/bug.php?id=53727 could have spotted this
    - our tests should have start failing after the change
    Third option:
    - RC testers might have spotted and reported it.

    I have the impression that very few people actually test these. When
    creating an RC we inform the "primary testers" as well as qa and
    internals list members. From there I get one or two responses in
    general.

    When I approach PHP users I often get answers like installing PHP
    without breaking their setup would be complicated (which is not the case
    but maybe needs education?) and they won't have time. I try to use a
    hypothetical case, like we have here in reality, to explain them why it
    is beneficial for their business if it is detected early as then we can
    fix it, fixing something after a release is hard. We also can try to
    improve our tests but we will never be able to test each and every way
    PHP is being used out there in the wild.


    So how can we motivate people to test new versions during RC not the day
    after it is being released?


    We don't push them out as news on the php.net frontpage and we don't
    send it out to the announce list for reasons like not confusing users.
    Should we change that? Other ideas?

    johannes
    agree, should have mentioned that.
    I think that currently testing the RCs have a very high barrier.
    usually they are going unnoticed for most people and compiling your
    own version (with all the extensions that you need) can be really
    cumbersome.
    - we need to get out the word to the masses (the current php.net site
    simply lacks this, maybe the http://prototype.php.net/ will be better
    in this regard), for which we also need to lower the barriers to
    entry:
    - better documentation about how to build your own php version would
    be a must, maybe phpfarm can be also useful for this
    I would really glad to have a script that will be eager to enable as
    much modules as it can find on my machine (plus enable debug flags as
    --enable-zts-maintainer and such) and even offer to install some
    missing dependencies from OS packages (I'm on ubuntu so it's easy to
    do some apt-get/yum install if you know what are you for). For now you
    have to read quite a long list of configure options and then install
    their dependencies which are quite non-obvious to find out.
    - we should cooperate with the major php projects out there to run
    their testsuites against the new releases or maybe even trunk, if I
    remember correctly somebody mentioned that we already do this for some
    projects (maybe Pierre mentioned this). this would be an easy way to
    boost our test coverage and make the BC breaks more obvious.
    - having pre-packaged versions of php available would also help,
    testing out the latest mysql versions are much more easy for example,
    as I can just grab the Linux - Generic archive, extract it, and voila,
    I can test it.
    - projects like http://apt.damz.org/ also help

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

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php


    --
    Regards,
    Shein Alexey
  • Christopher Jones at Aug 24, 2011 at 2:47 pm

    On 8/24/11 3:42 AM, Johannes Schlüter wrote:

    We don't push them out as news on the php.net frontpage and we don't
    send it out to the announce list for reasons like not confusing users.
    Should we change that?
    Announcements of RC X, Alpha X etc should be on the frontpage.

    Chris

    --
    Email: christopher.jones@oracle.com
    Tel: +1 650 506 8630
    Blog: http://blogs.oracle.com/opal/
  • Johannes Schlüter at Aug 24, 2011 at 2:59 pm

    On Wed, 2011-08-24 at 07:47 -0700, Christopher Jones wrote:
    On 8/24/11 3:42 AM, Johannes Schlüter wrote:

    We don't push them out as news on the php.net frontpage and we don't
    send it out to the announce list for reasons like not confusing users.
    Should we change that?
    Announcements of RC X, Alpha X etc should be on the frontpage.
    Provocative: Would that help in any way? I know that PEAR people knew
    about the RCs but still there were 3 RCs or so not properly tested with
    PEAR it seems.

    A hope there is that more visibility == more testing == more feedback.

    johannes

    ps. I'm not blaming PEAR or any individual. But well it is a php.net
    project and having an issue in there discovered roughly a week after
    release is late and is an indication of a fundamental problem ...
    Chris

    --
    Email: christopher.jones@oracle.com
    Tel: +1 650 506 8630
    Blog: http://blogs.oracle.com/opal/
  • Alan at Aug 24, 2011 at 9:37 am
    Since when has changing documented behaviour and breaking a large number
    of applications been acceptable? (Well, happens occasionally by accident ..)

    This was a 'feature change' to is_a(), it was documented as _only_
    returning 'TRUE' when an object was passed as the first object and it
    was an instance of that...

    I did read through the previous posts, (just caught up the other day),
    it looks like if anybody really need to do what this new feature
    provides, (which is probably very rare), then

    $left == $right || is_subclass_of($left,$right)

    should work fine without breaking any code.

    Regards
    Alan
    On Wednesday, August 24, 2011 03:44 PM, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 3:57 AM, alan@akbkhome.comwrote:
    It might have been better to have waited for the is_a() fix to get sorted
    out.. - It's a very annoying BC break (changes the documented behaviour),
    and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:

    as Stas pointed out:
    "Not a bug. $var is interpreted as a class name. To know if one class
    extends another, the class in question (first one) should be loaded.
    There's no need to load the second one since if it's unknown nothing
    can be instance of it and existing classes can not extend it."
    if you used this previously

    from Dmitry:
    "Before the patch, is_a() didn't accept string as the first argument
    at all, so it always returned "false" and never triggered
    __autoload(). The proposed patch didn't revert to original behavior.
    It just disables autoloading and may lead to wrong result.

    class a {}
    class b extends a {}
    var_dump(is_a("b", "a")); // it was false before 5.3.7, now it's true

    I would say that the old behaviour was wrong, especially because
    "instanceof" and is_subclass_of() already implemented support for
    string arguments."

    so your example was bogus, as passing a non-object as a first
    parameter wasn't supported (see http://php.net/is_a) so your code
    example depends on an undefined behavior and results in a bogus result
    (is_a() alwas returned false if you passed a non-object)

    so in a way it is really a BC, but I think that this change is really a bugfix.
  • Zeev Suraski at Aug 24, 2011 at 10:43 am
    I think there are two ways to look at it:

    - As a new feature. If I understand you correctly, the fact that beforehand is_a() was documented to only return TRUE in case the first argument was an instance of the second argument, means that if we do anything beyond that - e.g. support strings as the first argument - this means we break compatibility (please correct me if I misunderstood). IMHO that's not a realistic way to look at retaining compatibility, and I'm saying that as someone who's almost religious about maintain compatibility. With that view of the world, almost every bug fix and literally every feature we add - breaks compatibility.

    - As a bug fix. Strings were supported even before this change, but they weren't working properly or consistently with the way classes work everywhere else in PHP. That was fixed. Just so that we feel good about ourselves, we also changed undocumented behavior. In case it's not clear, a situation where is_a("bar", "foo") returns FALSE, even though bar extends foo, but bar simply doesn't happen to be loaded in memory at the time of the call - can lead to very real world bugs.

    Zeev
    -----Original Message-----
    From: alan@akbkhome.com
    Sent: Wednesday, August 24, 2011 12:37 PM
    To: internals@lists.php.net
    Subject: Re: [PHP-DEV] PHP 5.3.8 Released!

    Since when has changing documented behaviour and breaking a large
    number of applications been acceptable? (Well, happens occasionally by
    accident ..)

    This was a 'feature change' to is_a(), it was documented as _only_ returning
    'TRUE' when an object was passed as the first object and it was an instance of
    that...

    I did read through the previous posts, (just caught up the other day), it looks
    like if anybody really need to do what this new feature provides, (which is
    probably very rare), then

    $left == $right || is_subclass_of($left,$right)

    should work fine without breaking any code.

    Regards
    Alan
    On Wednesday, August 24, 2011 03:44 PM, Ferenc Kovacs wrote:
    On Wed, Aug 24, 2011 at 3:57 AM,
    alan@akbkhome.comwrote:
    It might have been better to have waited for the is_a() fix to get
    sorted out.. - It's a very annoying BC break (changes the documented
    behaviour), and now it means we need 4.3.9 in a few more days.

    Let me know if you need help testing / applying etc..
    from what I understand, this won't be changed, as the current behavior
    is correct, the old was a bug:

    as Stas pointed out:
    "Not a bug. $var is interpreted as a class name. To know if one class
    extends another, the class in question (first one) should be loaded.
    There's no need to load the second one since if it's unknown nothing
    can be instance of it and existing classes can not extend it."
    if you used this previously

    from Dmitry:
    "Before the patch, is_a() didn't accept string as the first argument
    at all, so it always returned "false" and never triggered
    __autoload(). The proposed patch didn't revert to original behavior.
    It just disables autoloading and may lead to wrong result.

    class a {}
    class b extends a {}
    var_dump(is_a("b", "a")); // it was false before 5.3.7, now it's true

    I would say that the old behaviour was wrong, especially because
    "instanceof" and is_subclass_of() already implemented support for
    string arguments."

    so your example was bogus, as passing a non-object as a first
    parameter wasn't supported (see http://php.net/is_a) so your code
    example depends on an undefined behavior and results in a bogus result
    (is_a() alwas returned false if you passed a non-object)

    so in a way it is really a BC, but I think that this change is really a bugfix.

    --
    PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit:
    http://www.php.net/unsub.php
  • Pierre Joye at Aug 24, 2011 at 11:23 am

    On Wed, Aug 24, 2011 at 12:43 PM, Zeev Suraski wrote:
    I think there are two ways to look at it:

    - As a new feature.  If I understand you correctly, the fact that beforehand is_a() was documented to only return TRUE in case the first argument was an instance of the second argument, means that if we do anything beyond that - e.g. support strings as the first argument - this means we break compatibility (please correct me if I misunderstood).  IMHO that's not a realistic way to look at retaining compatibility, and I'm saying that as someone who's almost religious about maintain compatibility.  With that view of the world, almost every bug fix and literally every feature we add - breaks compatibility.

    - As a bug fix.  Strings were supported even before this change, but they weren't working properly or consistently with the way classes work everywhere else in PHP.  That was fixed.  Just so that we feel good about ourselves, we also changed undocumented behavior.  In case it's not clear, a situation where is_a("bar", "foo") returns FALSE, even though bar extends foo, but bar simply doesn't happen to be loaded in memory at the time of the call - can lead to very real world bugs.
    No matter what it is or how it is defined by us, it breaks existing
    code and that should be avoided in bug fixes releases like 5.3.7/8.

    Cheers,
  • Zeev Suraski at Aug 24, 2011 at 11:41 am

    No matter what it is or how it is defined by us, it breaks existing code and that
    should be avoided in bug fixes releases like 5.3.7/8.
    Pierre,

    This wholesale statement doesn't get us anywhere. Every bug fix can result in breaking existing code. If due to a logic error, under some circumstances - file_exists() returned false for a file that actually exists, are we barred from fixing that in a maintenance release? Obviously not. What about bug # 54459? What if some piece of code out there relied on this behavior? What about # 55082 - what if someone already relied on this and wrote a layer to alter the output accordingly?

    Since clearly the definition of never breaking existing code, no matter how far-fetched it may be, means we can't do just about anything in bug fix releases - we need to set slightly more realistic definitions. The fix for is_a() falls squarely within the realm of stuff we should be doing in bug fix releases, IMHO. It is a bug fix, bug fixes by definition change behavior - sometimes to the degree of breaking certain (broken) pieces of code.

    Zeev
  • Pierre Joye at Aug 24, 2011 at 11:44 am

    On Wed, Aug 24, 2011 at 1:40 PM, Zeev Suraski wrote:
    No matter what it is or how it is defined by us, it breaks existing code and that
    should be avoided in bug fixes releases like 5.3.7/8.
    Pierre,

    This wholesale statement doesn't get us anywhere.
    It does, we underestimate the situation and this
    fix/improvement/consistency change breaks apps and codes out there.
    And I do not consider it as acceptable at this stage in 5.3.x. Let do
    it only in 5.4.
    Every bug fix can result in breaking existing code.
    Not every, and here it is easily fixable.


    Cheers,
  • Zeev Suraski at Aug 24, 2011 at 12:01 pm

    This wholesale statement doesn't get us anywhere.
    It does, we underestimate the situation and this fix/improvement/consistency
    change breaks apps and codes out there.
    And I do not consider it as acceptable at this stage in 5.3.x. Let do it only in
    5.4.

    How is it different from any of the other non-crash-fixing bugs we fixed, that could break apps that relied on the old behavior? Gave you two examples from the last release (well, 5.3.7), and another imaginary one. They all fall in the category of potentially breaking code out there, and yet - they should all be fixed.
    Every bug fix can result in breaking existing code.
    Not every, and here it is easily fixable.
    I'll refrain from word fighting you on the first part, but on the second - how is it easily fixable?

    This:

    is_a("bar", "foo"); // false
    $obj = new bar;
    is_a($obj, "foo"); // true, since now that bar's been loaded we know it extends foo

    Is a language bug, and a pretty bad one too.

    Zeev
  • Pierre Joye at Aug 24, 2011 at 12:12 pm

    On Wed, Aug 24, 2011 at 2:00 PM, Zeev Suraski wrote:
    This wholesale statement doesn't get us anywhere.
    It does, we underestimate the situation and this fix/improvement/consistency
    change breaks apps and codes out there.
    And I do not consider it as acceptable at this stage in 5.3.x. Let do it only in
    5.4.

    How is it different from any of the other non-crash-fixing bugs we fixed, that could break apps that relied on the old behavior? Gave you two examples from the last release (well, 5.3.7), and another imaginary one.  They all fall in the category of potentially breaking code out there, and yet - they should all be fixed.

    Zeev, the whole point is that changes like this one must be discussed
    on a case by case basis. It is now very well known that when a fix
    requires a test change, then it often leads to bc breaks or similar
    issues.

    I do not think we have to argue about the semantics or general cases
    but how to avoid such things and be sure that we break as less code as
    possible in patches release.

    It is also obvious, as you stated, that there will have edge cases
    where we have to break existing codes, for security or critical
    reasons. This is definitively not the case here.
  • Zeev Suraski at Aug 24, 2011 at 12:23 pm

    by case basis. It is now very well known that when a fix requires a test change,
    then it often leads to bc breaks or similar issues.

    I do not think we have to argue about the semantics or general cases but how
    to avoid such things and be sure that we break as less code as possible in
    patches release.
    In order to discuss how to avoid 'such things', we need to talk about different types of cases.
    It is also obvious, as you stated, that there will have edge cases where we have
    to break existing codes, for security or critical reasons. This is definitively not
    the case here.
    It has nothing to do with security (criticality is subjective so I'm leaving it aside). The 3 bugs I mentioned (2 from 5.3.7 and one imaginary) have nothing to do with security, and yet we fixed them (or would have fixed them), despite the potential for people out there relying on the old behavior. It boils down to evaluating the severity of the bug and the likelihood that it'll break code. If it's a clear bug, which IMHO this is_a() issue was - then unless we're looking at code breakage at massive scale, it should be fixed.

    Again, I'm almost religious about retaining compatibility (even across major versions), but if we had a piece of code that was returning clearly the wrong value, we can't ignore it because some (my guess very few but who knows) relied on this behavior.

    Zeev
  • Alan at Aug 24, 2011 at 12:38 pm
    " If it's a clear bug, which IMHO this is_a() issue was - then unless
    we're looking at code breakage at massive scale, it should be fixed. "

    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like
    that for > 8 years....

    google search for PEAR::isError shows 16,600 matches..

    http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&type=cs
    <http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&type=cs>

    for is_a you get 149K. and this is only public code...

    It's big... Luckily quite a few people are on holiday this month and
    will not upgrade too soon.

    Regards
    Alan


    On Wednesday, August 24, 2011 08:22 PM, Zeev Suraski wrote:

    It has nothing to do with security (criticality is subjective so I'm leaving it aside). The 3 bugs I mentioned (2 from 5.3.7 and one imaginary) have nothing to do with security, and yet we fixed them (or would have fixed them), despite the potential for people out there relying on the old behavior. It boils down to evaluating the severity of the bug and the likelihood that it'll break code. If it's a clear bug, which IMHO this is_a() issue was - then unless we're looking at code breakage at massive scale, it should be fixed.

    Again, I'm almost religious about retaining compatibility (even across major versions), but if we had a piece of code that was returning clearly the wrong value, we can't ignore it because some (my guess very few but who knows) relied on this behavior.

    Zeev
  • Pierre Joye at Aug 24, 2011 at 12:42 pm
    Hi Alan,

    the breakage is about is_a with a string as 1st argument, not is_a as
    a whole. So yes, it breaks is_a alone is used as validation.
    On Wed, Aug 24, 2011 at 2:38 PM, alan@akbkhome.com wrote:
    " If it's a clear bug, which IMHO this is_a() issue was - then unless we're
    looking at code breakage at massive scale, it should be fixed. "

    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like that
    for > 8 years....

    google search for PEAR::isError shows 16,600 matches..

    http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&type=cs
    <http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&type=cs>

    for is_a you get 149K. and this is only public code...

    It's big... Luckily quite a few people are on holiday this month and will
    not upgrade too soon.

    --
    Pierre

    @pierrejoye | http://blog.thepimp.net | http://www.libgd.org
  • Jonathan Bond-Caron at Sep 2, 2011 at 11:27 am

    On Wed Aug 24 08:42 AM, Pierre Joye wrote:
    Hi Alan,

    the breakage is about is_a with a string as 1st argument, not is_a as
    a whole. So yes, it breaks is_a alone is used as validation.
    I've been digging more into this:
    http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/Zend/zend_builtin_fun
    ctions.c?r1=307522&r2=312904&pathrev=312904

    From what I understand, this patch is only place where is_a() all of sudden
    starts accepting a string.
    Btw the documentation has never been updated:
    http://php.net/manual/en/function.is-a.php

    It seems unintentional, the patch tries to fix a bug but introduces a new
    'feature'. Should it be reverted?
  • Etienne Kneuss at Sep 2, 2011 at 11:33 am
    Hi,
    On Fri, Sep 2, 2011 at 13:26, Jonathan Bond-Caron wrote:
    On Wed Aug 24 08:42 AM, Pierre Joye wrote:
    Hi Alan,

    the breakage is about is_a with a string as 1st argument, not is_a as
    a whole. So yes, it breaks is_a alone is used as validation.
    I've been digging more into this:

    http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/Zend/zend_builtin_fun
    ctions.c?r1=307522&r2=312904&pathrev=312904

    From what I understand, this patch is only place where is_a() all of sudden
    starts accepting a string.
    Btw the documentation has never been updated:
    http://php.net/manual/en/function.is-a.php

    It seems unintentional, the patch tries to fix a bug but introduces a new
    'feature'. Should it be reverted?
    We already discussed that *in length* the past couple of weeks, the patch
    was in fact intentional and we decided not to revert it.

    Best,



    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php

    --
    Etienne Kneuss
    http://www.colder.ch
  • Nicolas Grekas at Sep 2, 2011 at 5:01 pm

    We already discussed that *in length* the past couple of weeks, the patch
    was in fact intentional and we decided not to revert it.
    I'm ok with the decision,
    but what about adding a third argument [, bool $autoload = true ] to is_a()
    and is_subclass_of(),
    in order to be consistent with class_parents() and class_implements() ?

    Nicolas
  • Zeev Suraski at Aug 24, 2011 at 12:51 pm
    Well, I have to admit this is mighty convincing :) Wasn't aware of this use-case. Falls under the category of mass breakage I guess.

    Zeev

    -----Original Message-----
    From: alan@akbkhome.com
    Sent: Wednesday, August 24, 2011 3:39 PM
    To: Zeev Suraski; internals@lists.php.net
    Subject: Re: [PHP-DEV] PHP 5.3.8 Released!

    " If it's a clear bug, which IMHO this is_a() issue was - then unless we're
    looking at code breakage at massive scale, it should be fixed. "

    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like that for > 8
    years....

    google search for PEAR::isError shows 16,600 matches..

    http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&
    type=cs
    <http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php
    &type=cs>

    for is_a you get 149K. and this is only public code...

    It's big... Luckily quite a few people are on holiday this month and will not
    upgrade too soon.

    Regards
    Alan


    On Wednesday, August 24, 2011 08:22 PM, Zeev Suraski wrote:

    It has nothing to do with security (criticality is subjective so I'm leaving it
    aside). The 3 bugs I mentioned (2 from 5.3.7 and one imaginary) have nothing
    to do with security, and yet we fixed them (or would have fixed them), despite
    the potential for people out there relying on the old behavior. It boils down to
    evaluating the severity of the bug and the likelihood that it'll break code. If it's
    a clear bug, which IMHO this is_a() issue was - then unless we're looking at
    code breakage at massive scale, it should be fixed.
    Again, I'm almost religious about retaining compatibility (even across major
    versions), but if we had a piece of code that was returning clearly the wrong
    value, we can't ignore it because some (my guess very few but who knows)
    relied on this behavior.
    Zeev
  • JJ at Aug 24, 2011 at 2:46 pm
    I think it's too late to do much of anything other than document the
    change in a way that makes it easier for people to recognize. I spoke
    to Philip about this offline and I think the two options along these
    lines are:

    1) Add a WARNING box which specifies the change for the function, that
    a true type check for strings no longer yields FALSE
    2) Update the first argument documentation from object -> mixed,
    mention it has started accepting mixed as of 5.3.7, and highlight the
    fact this function no longer returns FALSE when the type check (for
    strings) fails.

    fwiw - the pre-5.3.7 behavior was sub-optimal/broken but this is a BC
    break, if you consider modifying an existing function signature a BC
    break (which I'd hope most people do).

    I realize the release RFC isn't live till 5.4 but I am concerned that
    the continuing debate around what is and isn't a BC break, which has
    taken a large chunk of this thread, will hinder the relrfc moving
    forward.

    - JJ
    On Wed, Aug 24, 2011 at 5:50 AM, Zeev Suraski wrote:
    Well, I have to admit this is mighty convincing :)  Wasn't aware of this use-case.  Falls under the category of mass breakage I guess.

    Zeev

    -----Original Message-----
    From: alan@akbkhome.com
    Sent: Wednesday, August 24, 2011 3:39 PM
    To: Zeev Suraski; internals@lists.php.net
    Subject: Re: [PHP-DEV] PHP 5.3.8 Released!

    " If it's a clear bug, which IMHO this is_a() issue was - then unless we're
    looking at code breakage at massive scale, it should be fixed. "

    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like that for > 8
    years....

    google search for PEAR::isError shows 16,600 matches..

    http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php&
    type=cs
    <http://www.google.com/codesearch#search/&q=PEAR::isError%20lang:php
    &type=cs>

    for is_a you get 149K. and this is only public code...

    It's big... Luckily quite a few people are on holiday this month and will not
    upgrade too soon.

    Regards
    Alan


    On Wednesday, August 24, 2011 08:22 PM, Zeev Suraski wrote:

    It has nothing to do with security (criticality is subjective so I'm leaving it
    aside).  The 3 bugs I mentioned (2 from 5.3.7 and one imaginary) have nothing
    to do with security, and yet we fixed them (or would have fixed them), despite
    the potential for people out there relying on the old behavior. It boils down to
    evaluating the severity of the bug and the likelihood that it'll break code.  If it's
    a clear bug, which IMHO this is_a() issue was - then unless we're looking at
    code breakage at massive scale, it should be fixed.
    Again, I'm almost religious about retaining compatibility (even across major
    versions), but if we had a piece of code that was returning clearly the wrong
    value, we can't ignore it because some (my guess very few but who knows)
    relied on this behavior.
    Zeev

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
  • Stas Malyshev at Aug 24, 2011 at 5:01 pm
    Hi!
    On 8/24/11 5:38 AM, alan@akbkhome.com wrote:
    " If it's a clear bug, which IMHO this is_a() issue was - then unless
    we're looking at code breakage at massive scale, it should be fixed. "

    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like
    that for> 8 years....
    So, PEAR has a bug, if it passes non-object argument to is_a, as
    previously is_a was only documented (and actually still is) as accepting
    objects. The fact that this bug remained for 8 years is very sad, but it
    doesn't cease to be a bug.

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Lester Caine at Aug 24, 2011 at 8:45 pm

    Stas Malyshev wrote:
    mmh.. how much breakage did you want.

    PEAR::isError is basically is_a($input, 'PEAR_Error'); it's been like
    that for> 8 years....
    So, PEAR has a bug, if it passes non-object argument to is_a, as previously is_a
    was only documented (and actually still is) as accepting objects. The fact that
    this bug remained for 8 years is very sad, but it doesn't cease to be a bug.
    If I am reading things right, is_a was only designed to handle an object, so
    feeding it a mixed parameter in isError was always wrong? As far as I can see,
    on the whole, the PEAR code only ever feeds an object and feeding it a string
    would have to be a real error? So there are a number of actions here all of
    which are potentially wrong, and PEAR should return an error if $input is not a
    valid object rather than relying on undocumented actions simply to fail?

    --
    Lester Caine - G8HFL
    -----------------------------
    Contact - http://lsces.co.uk/wiki/?page=contact
    L.S.Caine Electronic Services - http://lsces.co.uk
    EnquirySolve - http://enquirysolve.com/
    Model Engineers Digital Workshop - http://medw.co.uk//
    Firebird - http://www.firebirdsql.org/index.php
  • Stas Malyshev at Aug 24, 2011 at 9:10 pm
    Hi!
    On 8/24/11 1:45 PM, Lester Caine wrote:
    If I am reading things right, is_a was only designed to handle an object, so
    feeding it a mixed parameter in isError was always wrong? As far as I can see,
    on the whole, the PEAR code only ever feeds an object and feeding it a string
    would have to be a real error? So there are a number of actions here all of
    which are potentially wrong, and PEAR should return an error if $input is not a
    valid object rather than relying on undocumented actions simply to fail?
    If they just wanted to see if it's an object of class PEAR_Error,
    instanceof should be used.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Ferenc Kovacs at Aug 24, 2011 at 9:16 pm

    If I am reading things right, is_a was only designed to handle an object, so
    feeding it a mixed parameter in isError was always wrong? As far as I can
    see, on the whole, the PEAR code only ever feeds an object and feeding it a
    string would have to be a real error? So there are a number of actions here
    all of which are potentially wrong, and PEAR should return an error if
    $input is not a valid object rather than relying on undocumented actions
    simply to fail?
    for the record, this is what happened:
    1, https://bugs.php.net/bug.php?id=53727 got reported
    2, dimitry commited http://svn.php.net/viewvc/?view=revision&amp;revision=312904
    3, that commit broke Pear, as is_a() started throwing a warning, if
    non-object is passed as first argument
    https://pear.php.net/bugs/bug.php?id=18656
    4, Johannes brought that issue up on the mailing list:
    http://www.mail-archive.com/internals@lists.php.net/msg51868.html and
    in the end the the accepted solution was to remove the warning.
    5, Helgi fixed Pear in the meanwhile
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313081&r2=313083&pathrev=313340
    6, Stas removes the warning
    http://svn.php.net/viewvc?view=revision&amp;revision=313162 Dmitry
    adds some tests
    http://svn.php.net/viewvc/?view=revision&amp;revision=313271
    7, Helgi reverts the Pear fix
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313340&r2=313339&pathrev=313340
    8, nobody notices the meaning of this change:
    http://svn.php.net/viewvc/php/php-src/branches/PHP_5_4/Zend/zend_builtin_functions.c?r1=312904&r2=312903&pathrev=312904#l848
    which AFAIK means that the zend_lookup_class (and hence autoloading)
    will always be called if the first argument is a string for is_a.
    previously it would only happen for is_subclass_of()

    Tyrael

    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Stas Malyshev at Aug 24, 2011 at 9:36 pm
    Hi!

    Thanks for providing the timeline.
    On 8/24/11 2:15 PM, Ferenc Kovacs wrote:
    5, Helgi fixed Pear in the meanwhile
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313081&r2=313083&pathrev=313340
    This fix doesn't look good - it doesn't do what is was meant to do.
    7, Helgi reverts the Pear fix
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313340&r2=313339&pathrev=313340
    And this should be using instanceof instead.
    8, nobody notices the meaning of this change:
    http://svn.php.net/viewvc/php/php-src/branches/PHP_5_4/Zend/zend_builtin_functions.c?r1=312904&r2=312903&pathrev=312904#l848
    which AFAIK means that the zend_lookup_class (and hence autoloading)
    will always be called if the first argument is a string for is_a.
    previously it would only happen for is_subclass_of()
    Well, it is obvious to me that is_a() and is_subclass_of() should work
    the same and both autoload the first argument if it's a string. However,
    the docs have is_subclass_of() documented as accepting string while
    is_a() is not and it worked as always returning false given non-object.
    I think we could easily keep this behavior for 5.3 even though I think
    relying on this is wrong (and you SHOULD fix it anywhere your code
    relies on it, including PEAR).
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Ferenc Kovacs at Aug 24, 2011 at 9:47 pm

    On Wed, Aug 24, 2011 at 11:33 PM, Stas Malyshev wrote:
    Hi!

    Thanks for providing the timeline.
    On 8/24/11 2:15 PM, Ferenc Kovacs wrote:

    5, Helgi fixed Pear in the meanwhile

    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313081&r2=313083&pathrev=313340
    This fix doesn't look good - it doesn't do what is was meant to do.
    /o\, I didn't noticed it until you mentioned...
    And this should be using instanceof instead.
    yep, I think the main reason that is_a was used in the first place is
    the fact that instanceof was added with php 5...
    obviously there is no point in watching out for php4 compatibility anymore.
    or at least it shouldn't be.


    --
    Ferenc Kovács
    @Tyr43l - http://tyrael.hu
  • Alan Knowles at Aug 24, 2011 at 11:39 pm
    Some real history for the young ones ;)

    If you go all the way back to when is_a() was introduced, it appears
    that it was done to simplify the code in PEAR::isError, which basically
    did stuff like

    if is_object() and is subclass() or get_classname() == ...

    So the previous behavior was very likely the 'designed' behavior. Not an
    accidental side effect, or bug.

    It's use case is reasonably common when doing tests on mixed returns
    (method returns PEAR:error, object or normal value.)

    So we had a situation where a reasonable number of people (eg. anyone
    who had used PEAR), had seen and expected the previous behavior.

    Where as now, we have not had a single direct bug report saying this
    behavior is bad (AFAIK), yet we are going to change it to fix an unusual
    use case on is_subclass_of as they use the same backend code.

    If this had been reported as a bug, or anybody on this list had actually
    been using it and had a gotcha moment, fine. but I've not heard anybody
    say that, just they think perhaps it should work a different way.

    Please do not fix something that is not broken, and breaks real working
    code, just for the hell of it, even in 5.4.

    Regards
    Alan
    On Thursday, August 25, 2011 05:33 AM, Stas Malyshev wrote:
    Hi!

    Thanks for providing the timeline.
    On 8/24/11 2:15 PM, Ferenc Kovacs wrote:
    5, Helgi fixed Pear in the meanwhile
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313081&r2=313083&pathrev=313340
    This fix doesn't look good - it doesn't do what is was meant to do.
    7, Helgi reverts the Pear fix
    http://svn.php.net/viewvc/pear/pear-core/tags/PEAR-1.9.5/PEAR.php?r1=313340&r2=313339&pathrev=313340
    And this should be using instanceof instead.
    8, nobody notices the meaning of this change:
    http://svn.php.net/viewvc/php/php-src/branches/PHP_5_4/Zend/zend_builtin_functions.c?r1=312904&r2=312903&pathrev=312904#l848

    which AFAIK means that the zend_lookup_class (and hence autoloading)
    will always be called if the first argument is a string for is_a.
    previously it would only happen for is_subclass_of()
    Well, it is obvious to me that is_a() and is_subclass_of() should work
    the same and both autoload the first argument if it's a string.
    However, the docs have is_subclass_of() documented as accepting string
    while is_a() is not and it worked as always returning false given
    non-object. I think we could easily keep this behavior for 5.3 even
    though I think relying on this is wrong (and you SHOULD fix it
    anywhere your code relies on it, including PEAR).
  • Stas Malyshev at Aug 25, 2011 at 1:10 am
    Hi!
    On 8/24/11 4:38 PM, Alan Knowles wrote:
    Some real history for the young ones ;)
    I wonder who you are meaning... :)
    So the previous behavior was very likely the 'designed' behavior. Not an
    accidental side effect, or bug.
    Bugs can be very well intentional, but if they use the language wrong
    way they are bugs.
    It's use case is reasonably common when doing tests on mixed returns
    (method returns PEAR:error, object or normal value.)
    That's when you use instanceof.
    So we had a situation where a reasonable number of people (eg. anyone
    who had used PEAR), had seen and expected the previous behavior.
    Seeing wrong code somewhere doesn't mean it's not wrong. There's a
    reason we have the manual.
    Please do not fix something that is not broken, and breaks real working
    code, just for the hell of it, even in 5.4.
    is_a() was broken - it was returning different results from essentially
    the same function is_subclass_of() for no reason at all (no, somebody
    writing buggy code in PEAR using undocumented parameter types is not a
    reason). The reason why we kept is_a() and not killed it in favor of
    instanceof was to have it work with string arguments, since instanceof
    does not. Thus, string arguments should be handled properly. I can see
    how it can be argued that 5.3 is mature enough so such changes shouldn't
    be there, however correct in theory. For 5.4, I see no base for argument
    here.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Alan at Aug 25, 2011 at 1:51 am
    I'm not sure it's possible to get agreement on if this is a bug or not,
    you might see a bug, I just see this as a pointless change for
    consistency that pretty much nobody will ever need or use.

    I think I'll leave it as
    a) no bug was ever reported on the previous behaviour.

    b) intended "design" of is_subclass_of was originally to return false
    on non-object - andrei (1999)
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272>

    c) is_a() was introduced by sebastian (2002) and kept this intended
    behaviour
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234>

    d) when andrey (2004) proposed the change to accepts strings on
    is_subclass_of, he deliberately maintained the existing behaviour for
    is_a()
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349>

    e) is_a() has a valid use case , and is currently reasonably commonly used.

    d) the new behaviour is an uncommon use case, and can be done very
    easily in other ways.


    BTW. we could really do with a searchable archive of php.dev +
    internals... - It's pretty difficult to find out if this was ever
    discussed before..

    Regards
    Alan



    On Thursday, August 25, 2011 09:10 AM, Stas Malyshev wrote:
    Hi!
    On 8/24/11 4:38 PM, Alan Knowles wrote:
    Some real history for the young ones ;)
    I wonder who you are meaning... :)
    So the previous behavior was very likely the 'designed' behavior. Not an
    accidental side effect, or bug.
    Bugs can be very well intentional, but if they use the language wrong
    way they are bugs.
    It's use case is reasonably common when doing tests on mixed returns
    (method returns PEAR:error, object or normal value.)
    That's when you use instanceof.
    So we had a situation where a reasonable number of people (eg. anyone
    who had used PEAR), had seen and expected the previous behavior.
    Seeing wrong code somewhere doesn't mean it's not wrong. There's a
    reason we have the manual.
    Please do not fix something that is not broken, and breaks real working
    code, just for the hell of it, even in 5.4.
    is_a() was broken - it was returning different results from
    essentially the same function is_subclass_of() for no reason at all
    (no, somebody writing buggy code in PEAR using undocumented parameter
    types is not a reason). The reason why we kept is_a() and not killed
    it in favor of instanceof was to have it work with string arguments,
    since instanceof does not. Thus, string arguments should be handled
    properly. I can see how it can be argued that 5.3 is mature enough so
    such changes shouldn't be there, however correct in theory. For 5.4, I
    see no base for argument here.
  • Ronald Chmara at Aug 25, 2011 at 3:59 am

    On Wed, Aug 24, 2011 at 6:51 PM, alan@akbkhome.com wrote:
    BTW. we could really do with a searchable archive of php.dev + internals...
    - It's pretty difficult to find out if this was ever discussed before..
    http://marc.info/?l=php-internals

    marc has a ton of the PHP lists. (Is this not what you are looking for?)

    -Ronabop
  • Alan at Aug 25, 2011 at 4:05 am
    It did not like my search for is_a ;) - I guess it's too short.
    On Thursday, August 25, 2011 11:59 AM, Ronald Chmara wrote:
    On Wed, Aug 24, 2011 at 6:51 PM, alan@akbkhome.comwrote:
    BTW. we could really do with a searchable archive of php.dev + internals...
    - It's pretty difficult to find out if this was ever discussed before..
    http://marc.info/?l=php-internals

    marc has a ton of the PHP lists. (Is this not what you are looking for?)

    -Ronabop
  • Aleksandr Galkin at Aug 25, 2011 at 4:58 am
    For the record, I'd like to point out I do need the new behaviour.
    In 5.3.6 you need reflection to check if a class implements an interface.
    You also need to check is_subclass_of AND compare the lowercased classes.
    All that is about 5 times slower than is_a in 5.3.8.

    Probably it should be class_is_a() instead of altering is_a() behaviour,
    but the need to match class names against each other is pretty much real.

    Sincerely yours, Aleksandr.
    On Aug 25, 2011, at 05:51, alan@akbkhome.com wrote:

    I'm not sure it's possible to get agreement on if this is a bug or not, you might see a bug, I just see this as a pointless change for consistency that pretty much nobody will ever need or use.

    I think I'll leave it as
    a) no bug was ever reported on the previous behaviour.

    b) intended "design" of is_subclass_of was originally to return false on non-object - andrei (1999)
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272 <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272>

    c) is_a() was introduced by sebastian (2002) and kept this intended behaviour
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234 <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234>

    d) when andrey (2004) proposed the change to accepts strings on is_subclass_of, he deliberately maintained the existing behaviour for is_a()
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349 <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349>

    e) is_a() has a valid use case , and is currently reasonably commonly used.

    d) the new behaviour is an uncommon use case, and can be done very easily in other ways.


    BTW. we could really do with a searchable archive of php.dev + internals... - It's pretty difficult to find out if this was ever discussed before..

    Regards
    Alan
  • Pierre Joye at Aug 25, 2011 at 8:30 am

    On Thu, Aug 25, 2011 at 3:51 AM, alan@akbkhome.com wrote:

    BTW. we could really do with a searchable archive of php.dev + internals...
    - It's pretty difficult to find out if this was ever discussed before..
    Again, it was discussed already and the argument of using instanceof
    was used to deprecate is_a (along other arguments). I see no point to
    argue again about that. It was a mistake to change it again in 5.3,
    Zeev realized the use in this exact case, let move on now. Revert that
    in 5.3 and do it only in 5.4.
  • Zeev Suraski at Aug 25, 2011 at 8:39 am

    -----Original Message-----
    From: Pierre Joye
    Sent: Thursday, August 25, 2011 11:31 AM
    To: alan@akbkhome.com
    Cc: Stas Malyshev; internals@lists.php.net
    Subject: Re: [PHP-DEV] PHP 5.3.8 Released!

    On Thu, Aug 25, 2011 at 3:51 AM, alan@akbkhome.com
    wrote:
    BTW. we could really do with a searchable archive of php.dev + internals...
    - It's pretty difficult to find out if this was ever discussed before..
    Again, it was discussed already and the argument of using instanceof was used
    to deprecate is_a (along other arguments). I see no point to argue again about
    that. It was a mistake to change it again in 5.3, Zeev realized the use in this
    exact case, let move on now. Revert that in 5.3 and do it only in 5.4.
    Just so that my position is clear on the matter:
    - I still think that is_a() is working properly the way it does now, after the change.
    - I think the code in isError() is wrong, and should be fixed.
    - Had we (the collective we) known that fixing is_a() would result in such breakage, it would have probably been wise not to fix it in 5.3.x, and wait for 5.4 for this purpose.
    - Given that we've already done it - I wouldn't revert it. Fix it in PEAR. That's the only way to create something that works across all versions of 5.3.x.

    Zeev
  • Pierre Joye at Aug 25, 2011 at 8:43 am

    On Thu, Aug 25, 2011 at 10:39 AM, Zeev Suraski wrote:
    -----Original Message-----
    From: Pierre Joye
    Sent: Thursday, August 25, 2011 11:31 AM
    To: alan@akbkhome.com
    Cc: Stas Malyshev; internals@lists.php.net
    Subject: Re: [PHP-DEV] PHP 5.3.8 Released!

    On Thu, Aug 25, 2011 at 3:51 AM, alan@akbkhome.com
    wrote:
    BTW. we could really do with a searchable archive of php.dev + internals...
    - It's pretty difficult to find out if this was ever discussed before..
    Again, it was discussed already and the argument of using instanceof was used
    to deprecate is_a (along other arguments). I see no point to argue again about
    that. It was a mistake to change it again in 5.3, Zeev realized the use in this
    exact case, let move on now. Revert that in 5.3 and do it only in 5.4.
    Just so that my position is clear on the matter:
    - I still think that is_a() is working properly the way it does now, after the change.
    - I think the code in isError() is wrong, and should be fixed.
    - Had we (the collective we) known that fixing is_a() would result in such breakage, it would have probably been wise not to fix it in 5.3.x, and wait for 5.4 for this purpose.
    - Given that we've already done it - I wouldn't revert it. Fix it in PEAR.  That's the only way to create something that works across all versions of 5.3.x.
    I won't battle endlessly to get that fixed back again (you will always
    find smtg else to keep that thing in anyway ;-) but it does confirm
    what I said earlier about changing behaviors in patch releases. This
    is something we must deal with much more carefully. And using x.y.0
    tests as base is a good start, also the BC breakages are now covered
    by the RFC, let stick to that for 5.4.x.


    Cheers,
  • Johannes Schlüter at Aug 25, 2011 at 8:44 am

    On Thu, 2011-08-25 at 08:39 +0000, Zeev Suraski wrote:
    - Given that we've already done it - I wouldn't revert it. Fix it in
    PEAR. That's the only way to create something that works across all
    versions of 5.3.x.
    Unfortunately this is the case.

    johannes
  • Matthew Weier O'Phinney at Aug 25, 2011 at 1:32 pm

    On 2011-08-25, "alan@akbkhome.com" wrote:
    I'm not sure it's possible to get agreement on if this is a bug or
    not, you might see a bug, I just see this as a pointless change for
    consistency that pretty much nobody will ever need or use.
    Please don't generalize based on your own opinions and use cases. I am a
    long time PEAR user (and contributor), and I actually agree with the
    change.

    The reporter of the issue that started this all is a colleague of mine,
    Ralph Schindler, and we discussed it in June along with David Zuilke,
    who had run into similar issues we had (as well as discussed it with
    others in other projects). It's not an isolated request; there are many
    who find the current behavior of is_a() (pre-5.3.7) incoherent for
    modern PHP usage.

    Basically, in our case, we were building a DI container. To keep the DI
    container light-weight, you create definitions that utilize string class
    names. In order to determine what injections need to be made, you need
    to introspect the class a little -- and this is where is_a() vs
    is_subclass_of() vs instanceof comes into play. The latter two _require_
    an object instance -- which we may not yet be ready to provide (we may
    be trying to determine what to inject into the constructor). is_a() does
    _not_ require an object instance... but prior to 5.3.7 was unable to
    test against inherited behavior. As such, the only fallback is the much
    more expensive Reflection API.

    Having is_a() work properly with string class names and checking the
    inheritance hierarchy is a huge improvement, keeps it semantically
    consistent with the rest of the language, and provides capabilities
    is_subclass_of() cannot (as it cannot check against strings).
    I think I'll leave it as
    a) no bug was ever reported on the previous behaviour.
    False -- others in this thread have pointed it out, and I alluded to the
    report Ralph issued earlier.
    b) intended "design" of is_subclass_of was originally to return false
    on non-object - andrei (1999)
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=17245&r2=17272>

    c) is_a() was introduced by sebastian (2002) and kept this intended
    behaviour
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=67102&r2=69234>

    d) when andrey (2004) proposed the change to accepts strings on
    is_subclass_of, he deliberately maintained the existing behaviour for
    is_a()
    http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349
    <http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_builtin_functions.c?r1=170604&r2=171349>

    e) is_a() has a valid use case , and is currently reasonably commonly used.

    d) the new behaviour is an uncommon use case, and can be done very
    easily in other ways.


    BTW. we could really do with a searchable archive of php.dev +
    internals... - It's pretty difficult to find out if this was ever
    discussed before..

    Regards
    Alan



    On Thursday, August 25, 2011 09:10 AM, Stas Malyshev wrote:
    Hi!
    On 8/24/11 4:38 PM, Alan Knowles wrote:
    Some real history for the young ones ;)
    I wonder who you are meaning... :)
    So the previous behavior was very likely the 'designed' behavior. Not an
    accidental side effect, or bug.
    Bugs can be very well intentional, but if they use the language wrong
    way they are bugs.
    It's use case is reasonably common when doing tests on mixed returns
    (method returns PEAR:error, object or normal value.)
    That's when you use instanceof.
    So we had a situation where a reasonable number of people (eg. anyone
    who had used PEAR), had seen and expected the previous behavior.
    Seeing wrong code somewhere doesn't mean it's not wrong. There's a
    reason we have the manual.
    Please do not fix something that is not broken, and breaks real working
    code, just for the hell of it, even in 5.4.
    is_a() was broken - it was returning different results from
    essentially the same function is_subclass_of() for no reason at all
    (no, somebody writing buggy code in PEAR using undocumented parameter
    types is not a reason). The reason why we kept is_a() and not killed
    it in favor of instanceof was to have it work with string arguments,
    since instanceof does not. Thus, string arguments should be handled
    properly. I can see how it can be argued that 5.3 is mature enough so
    such changes shouldn't be there, however correct in theory. For 5.4, I
    see no base for argument here.

    --
    Matthew Weier O'Phinney
    Project Lead | matthew@zend.com
    Zend Framework | http://framework.zend.com/
    PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Related Discussions

People

Translate

site design / logo © 2022 Grokbase