FAQ
Hi,

I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.

-John

Search Discussions

  • Rasmus Lerdorf at May 23, 2014 at 1:28 am

    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

    I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than just
    adding support for Bison 3.

    -Rasmus
  • John Bafford at May 23, 2014 at 1:44 am
    Hi Rasmus,
    On May 22, 2014, at 20:28, Rasmus Lerdorf wrote:
    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

    I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than just
    adding support for Bison 3.

    -Rasmus
    I replaced %pure-parser (which was deprecated) with %pure_parser (its replacement). (Also, YYERROR_VERBOSE, which also was deprecated, with %error-verbose.)

    Most of the changes in the PR revolve around this change in Zend/zend_language_parser.y:
    -%code requires {
    -#ifdef ZTS
    -# define YYPARSE_PARAM tsrm_ls
    -# define YYLEX_PARAM tsrm_ls
    -#endif
    -}
    +%parse-param { void *tsrm_ls }
    +%lex-param { void *tsrm_ls }

    YYPARSE_PARAM and YYLEX_PARAM were deprecated, and then removed, and replaced with %parse-param and %lex-param. However, you cannot (or I did not see a way to) conditionally define parameters, as PHP was previously doing here.

    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still resolve to a parameter (that must always be present), which is passed NULL.

    -John
  • Stas Malyshev at May 23, 2014 at 5:46 am
    Hi!
    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in
    the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still
    resolve to a parameter (that must always be present), which is passed
    NULL.
    I don't think it is a very good idea to add extra parameter that does
    nothing to every function. There should be a better solution to this.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • John Bafford at May 23, 2014 at 8:42 am
    Hi Stas,
    On May 23, 2014, at 00:46, Stas Malyshev wrote:

    Hi!
    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in
    the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still
    resolve to a parameter (that must always be present), which is passed
    NULL.
    I don't think it is a very good idea to add extra parameter that does
    nothing to every function. There should be a better solution to this.
    I agree it would be ideal to not have to include the extra parameter in non-ZTS builds. However, it is only a few functions (zenderror, zendlex, zendparse, init_error, ini_parse, ini_lex) that gain the extra parameter. Those functions should not be frequently called, and when they are, a NULL (or actual local parameter TSRMLS) is passed, so the overhead should be pretty minimal.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
    -John
  • Jakub Zelenka at May 23, 2014 at 6:48 pm

    On Fri, May 23, 2014 at 6:46 AM, Stas Malyshev wrote:

    Hi!
    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in
    the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still
    resolve to a parameter (that must always be present), which is passed
    NULL.
    I don't think it is a very good idea to add extra parameter that does
    nothing to every function. There should be a better solution to this.
    I agree with Stas that the way how it's done at the moment is pretty ugly.

    I was dealing with similar problem in jsond parser (
    https://github.com/bukka/php-jsond/blob/master/jsond_parser.y ) where I
    decided to use a structure (php_json_parser) for general parser info as
    well as scanner data and TSRMLS if ZTS defined. It means that data from the
    scanner are passed to the parser through that structure instead of using
    global variable (that's the way how it's done in PHP). tsrm_ls can be set
    using TSRMLS_SET_CTX fetched from that structure TSRMLS_FETCH_FROM_CTX.

    So to sum it up... I think that a better solution would be using a
    structure that would contain scanner data (_zend_php_scanner_globals ) and
    TSRMLS if ZTS defined. It would be a cleaner solution IMHO. It could also
    allow some optimization for the compiler as local variables might be saved
    in registers (not sure about this one... :) ).

    However this is probaly more something for phpng as it requires a bit more
    changes... :)

    Jakub
  • Pierre Joye at May 23, 2014 at 6:40 am
    Hi John,
    On Fri, May 23, 2014 at 3:44 AM, John Bafford wrote:
    Hi Rasmus,
    On May 22, 2014, at 20:28, Rasmus Lerdorf wrote:
    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

    I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than just
    adding support for Bison 3.

    -Rasmus
    I replaced %pure-parser (which was deprecated) with %pure_parser (its replacement). (Also, YYERROR_VERBOSE, which also was deprecated, with %error-verbose.)

    Most of the changes in the PR revolve around this change in Zend/zend_language_parser.y:
    -%code requires {
    -#ifdef ZTS
    -# define YYPARSE_PARAM tsrm_ls
    -# define YYLEX_PARAM tsrm_ls
    -#endif
    -}
    +%parse-param { void *tsrm_ls }
    +%lex-param { void *tsrm_ls }

    YYPARSE_PARAM and YYLEX_PARAM were deprecated, and then removed, and replaced with %parse-param and %lex-param. However, you cannot (or I did not see a way to) conditionally define parameters, as PHP was previously doing here.

    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still resolve to a parameter (that must always be present), which is passed NULL.
    Thanks for your work, always good to support less archaic version of bison.

    Some comments:

    - does it still work with previous versions? If yes, do you know which
    or which minimum version we should then require?
    - 5.5 is stable, I am not sure it is a good idea to do these changes
    there. 5.6 may be a good candidate (but has to be done quickly, RMs
    can confirm if it is still possible)

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • John Bafford at May 23, 2014 at 8:25 am
    Pierre,
    On May 23, 2014, at 01:40, Pierre Joye wrote:

    Hi John,
    On Fri, May 23, 2014 at 3:44 AM, John Bafford wrote:
    Hi Rasmus,
    On May 22, 2014, at 20:28, Rasmus Lerdorf wrote:
    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

    I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than just
    adding support for Bison 3.

    -Rasmus
    I replaced %pure-parser (which was deprecated) with %pure_parser (its replacement). (Also, YYERROR_VERBOSE, which also was deprecated, with %error-verbose.)

    Most of the changes in the PR revolve around this change in Zend/zend_language_parser.y:
    -%code requires {
    -#ifdef ZTS
    -# define YYPARSE_PARAM tsrm_ls
    -# define YYLEX_PARAM tsrm_ls
    -#endif
    -}
    +%parse-param { void *tsrm_ls }
    +%lex-param { void *tsrm_ls }

    YYPARSE_PARAM and YYLEX_PARAM were deprecated, and then removed, and replaced with %parse-param and %lex-param. However, you cannot (or I did not see a way to) conditionally define parameters, as PHP was previously doing here.

    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still resolve to a parameter (that must always be present), which is passed NULL.
    Thanks for your work, always good to support less archaic version of bison.

    Some comments:

    - does it still work with previous versions? If yes, do you know which
    or which minimum version we should then require?
    - 5.5 is stable, I am not sure it is a good idea to do these changes
    there. 5.6 may be a good candidate (but has to be done quickly, RMs
    can confirm if it is still possible)
    I did test this with PHP 5.4 using bison 2.3, and it worked, but I elected to not base the patch on PHP 5.4 on Derick’s advice, as it could potentially remove compatibility with some or all Bison 1.x versions, which PHP 5.4 explicitly supports and which I did not really want to spend time testing. PHP 5.5 and master already require bison >= 2.4, so that was the earliest version I tested on those branches. For master and 5.5, I specifically tested 2.4, 2.7.12, and 3.0.2, and all three worked (tests passed) on both zts and non-zts builds. So, this PR, for 5.5 and (appropriately updated for) 5.6, only expands the supported versions of bison.
    Cheers,
    --
    Pierre
    -John
  • Julien Pauli at May 23, 2014 at 11:59 am

    On Fri, May 23, 2014 at 8:40 AM, Pierre Joye wrote:
    Hi John,
    On Fri, May 23, 2014 at 3:44 AM, John Bafford wrote:
    Hi Rasmus,
    On May 22, 2014, at 20:28, Rasmus Lerdorf wrote:
    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add support for Bison 3.0 to PHP 5.5.

    I’ve run the tests on this patch, with no unexpected errors, with Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and without --enable-maintainer-zts against the current PHP-5.5 and (with minor changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than just
    adding support for Bison 3.

    -Rasmus
    I replaced %pure-parser (which was deprecated) with %pure_parser (its replacement). (Also, YYERROR_VERBOSE, which also was deprecated, with %error-verbose.)

    Most of the changes in the PR revolve around this change in Zend/zend_language_parser.y:
    -%code requires {
    -#ifdef ZTS
    -# define YYPARSE_PARAM tsrm_ls
    -# define YYLEX_PARAM tsrm_ls
    -#endif
    -}
    +%parse-param { void *tsrm_ls }
    +%lex-param { void *tsrm_ls }

    YYPARSE_PARAM and YYLEX_PARAM were deprecated, and then removed, and replaced with %parse-param and %lex-param. However, you cannot (or I did not see a way to) conditionally define parameters, as PHP was previously doing here.

    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in the ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still resolve to a parameter (that must always be present), which is passed NULL.
    Thanks for your work, always good to support less archaic version of bison.

    Some comments:

    - does it still work with previous versions? If yes, do you know which
    or which minimum version we should then require?
    - 5.5 is stable, I am not sure it is a good idea to do these changes
    there. 5.6 may be a good candidate (but has to be done quickly, RMs
    can confirm if it is still possible)
    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.

    For 5.6, we are actually in RC stage (with first RC to come soon).
    Seeing the comments saying that it's all compatible and tests have
    passed, I'm not against requiring bison 3.x for PHP 5.6.
    Waiting for Ferenc's answer ;-)

    Thank you for your patch and your support to make PHP better.

    Julien Pauli
  • John Bafford at May 23, 2014 at 1:03 pm
    Hi Julien and Ferenc,
    On May 23, 2014, at 06:58, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.

    For 5.6, we are actually in RC stage (with first RC to come soon).
    Seeing the comments saying that it's all compatible and tests have
    passed, I'm not against requiring bison 3.x for PHP 5.6.
    Waiting for Ferenc's answer ;-)

    Thank you for your patch and your support to make PHP better.
    For your convenience, I’ve made a pull request for these changes against master (which would likely prevent at least one merge conflict vs. merging the 5.5-based pull request into master):

    https://github.com/php/php-src/pull/685

    I would point out that both pull requests only add Bison 3.x support; they don’t prevent Bison 2.x from working, so unless you have another reason for wanting to remove Bison 2.x support, this change doesn’t require dropping it. (I wouldn’t necessarily advocate dropping Bison 2.x support since we’re not actually using any Bison 3-specific features.).

    Julien Pauli
    -John
  • Ferenc Kovacs at May 23, 2014 at 5:29 pm
    2014.05.23. 13:59, "Julien Pauli" <jpauli@php.net> ezt írta:
    On Fri, May 23, 2014 at 8:40 AM, Pierre Joye wrote:
    Hi John,
    On Fri, May 23, 2014 at 3:44 AM, John Bafford wrote:
    Hi Rasmus,
    On May 22, 2014, at 20:28, Rasmus Lerdorf wrote:
    On 5/22/14, 6:01 PM, John Bafford wrote:
    Hi,

    I’ve submitted a PR (https://github.com/php/php-src/pull/683) to add
    support for Bison 3.0 to PHP 5.5.
    I’ve run the tests on this patch, with no unexpected errors, with
    Bison 2.4 (minimum version for PHP 5.5 and 5.6), 2.7, and 3.0, with and
    without --enable-maintainer-zts against the current PHP-5.5 and (with minor
    changes) master branches.
    Could you explain your changes a bit? You removed %pure-parser, for
    example, and you shuffled globals around. This looks like more than
    just
    adding support for Bison 3.

    -Rasmus
    I replaced %pure-parser (which was deprecated) with %pure_parser (its
    replacement). (Also, YYERROR_VERBOSE, which also was deprecated, with
    %error-verbose.)
    Most of the changes in the PR revolve around this change in
    Zend/zend_language_parser.y:
    -%code requires {
    -#ifdef ZTS
    -# define YYPARSE_PARAM tsrm_ls
    -# define YYLEX_PARAM tsrm_ls
    -#endif
    -}
    +%parse-param { void *tsrm_ls }
    +%lex-param { void *tsrm_ls }

    YYPARSE_PARAM and YYLEX_PARAM were deprecated, and then removed, and
    replaced with %parse-param and %lex-param. However, you cannot (or I did
    not see a way to) conditionally define parameters, as PHP was previously
    doing here.
    So instead, I created the TSRMLS_DN and TSRMLS_CN macros which, in the
    ZTS case, work like TSRMLS_D/TSRMLS_C, but when ZTS is off, still resolve
    to a parameter (that must always be present), which is passed NULL.
    Thanks for your work, always good to support less archaic version of
    bison.
    Some comments:

    - does it still work with previous versions? If yes, do you know which
    or which minimum version we should then require?
    - 5.5 is stable, I am not sure it is a good idea to do these changes
    there. 5.6 may be a good candidate (but has to be done quickly, RMs
    can confirm if it is still possible)
    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.

    For 5.6, we are actually in RC stage (with first RC to come soon).
    Seeing the comments saying that it's all compatible and tests have
    passed, I'm not against requiring bison 3.x for PHP 5.6.
    Waiting for Ferenc's answer ;-)

    Thank you for your patch and your support to make PHP better.

    Julien Pauli
    I would be fine with this change, even it happens after the first RC, given
    that it could only affect people, who are building php from source and not
    using the official release tarballs.
  • Pierre Joye at May 23, 2014 at 5:31 pm

    On May 23, 2014 7:28 PM, "Ferenc Kovacs" wrote:
    I would be fine with this change, even it happens after the first RC,
    given that it could only affect people, who are building php from source
    and not using the official release tarballs.

    I think it is fine for 5.6 as well.

    However issues can also happen in the generated files. We experienced that
    a couple of times already.

    Cheers,
    Pierre
  • Julien Pauli at May 23, 2014 at 5:48 pm

    On Fri, May 23, 2014 at 7:31 PM, Pierre Joye wrote:
    On May 23, 2014 7:28 PM, "Ferenc Kovacs" wrote:

    I would be fine with this change, even it happens after the first RC,
    given that it could only affect people, who are building php from source and
    not using the official release tarballs.
    I think it is fine for 5.6 as well.

    However issues can also happen in the generated files. We experienced that a
    couple of times already.
    We were talking about 5.6 ;-)

    I think it is fine. We'll have several RC to test that anyway.
    I don't consider that as beeing a huge critical change, so it's all
    right for 5.6.

    Who does the merge ? :-)

    Julien Pauli
  • Derick Rethans at May 28, 2014 at 11:41 am

    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.

    cheers,
    Derick

    --
    http://derickrethans.nl | http://xdebug.org
    Like Xdebug? Consider a donation: http://xdebug.org/donate.php
    twitter: @derickr and @xdebug
    Posted with an email client that doesn't mangle email: alpine
  • Jakub Zelenka at May 28, 2014 at 12:56 pm

    On Wed, May 28, 2014 at 12:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.
    Why not just disable checking of bison version in stable versions. We
    distribute generated tab files and there is no point to fail the configure
    step just because someone cannot create tab file. The bison is used only if
    the *.y files are changed. The thing is that such change is not allowed for
    stable branches anyway. In case someone really wants to play with grammar
    on old branches, then he/she should be able to install a correct version of
    bison. But I think that we can count number of such users on one hand.

    Just disabling version check is only config change and should not be a
    problem for stable branches IMHO.

    We can add a proper support for bison3 to phpng. The current patch seems to
    me more like a hack than a proper solution...

    Thoughts?

    Jakub
  • Jakub Zelenka at May 28, 2014 at 1:05 pm

    On Wed, May 28, 2014 at 1:56 PM, Jakub Zelenka wrote:
    On Wed, May 28, 2014 at 12:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.
    Why not just disable checking of bison version in stable versions. We
    distribute generated tab files and there is no point to fail the configure
    step just because someone cannot create tab file. The bison is used only if
    the *.y files are changed. The thing is that such change is not allowed for
    stable branches anyway. In case someone really wants to play with grammar
    on old branches, then he/she should be able to install a correct version of
    bison. But I think that we can count number of such users on one hand.

    Just disabling version check is only config change and should not be a
    problem for stable branches IMHO.

    We can add a proper support for bison3 to phpng. The current patch seems
    to me more like a hack than a proper solution...

    Thoughts?

    Jakub

    Ooops. We don't actaully distribute tab files. Thought that we do... :)

    Wouldn't be better to add them to the git? What's the actual reason to not
    to add them. The same thing we do for scanner files already. I'm not aware
    that bison would generate some platform specific tab files...?

    Regards

    Jakub
  • Johannes Schlüter at May 28, 2014 at 1:14 pm

    On Wed, 2014-05-28 at 14:05 +0100, Jakub Zelenka wrote:
    Ooops. We don't actaully distribute tab files. Thought that we do... :)

    Wouldn't be better to add them to the git? What's the actual reason to not
    to add them. The same thing we do for scanner files already. I'm not aware
    that bison would generate some platform specific tab files...?
    We don't in git ut snapshots and releases. Git is only for developers.
    Having generated files in version control is questionable as this leads
    to annoyance when timestamps come out wrong they are recreated using a
    different tool version and then are being committed or have conflicts
    "all the time".

    johannes
  • Jakub Zelenka at May 28, 2014 at 6:39 pm

    On Wed, May 28, 2014 at 2:14 PM, Johannes Schlüter wrote:
    On Wed, 2014-05-28 at 14:05 +0100, Jakub Zelenka wrote:
    Ooops. We don't actaully distribute tab files. Thought that we do... :)

    Wouldn't be better to add them to the git? What's the actual reason to not
    to add them. The same thing we do for scanner files already. I'm not aware
    that bison would generate some platform specific tab files...?
    We don't in git ut snapshots and releases. Git is only for developers.
    Having generated files in version control is questionable as this leads
    to annoyance when timestamps come out wrong they are recreated using a
    different tool version and then are being committed or have conflicts
    "all the time".
    That would make sense if we didn't have re2c generated files in the repo
    already. Anyway I see your point. Bison version varies across the systems
    more than re2c so the diffs in files would be more often which is not a
    good idea...

    The solution could be to stick just with one version of bison for people
    that want to do changes to the grammar. It should be possible fail the make
    phase (not configuration) when the version is different (for example empty
    YACC variable when the version is different and then add check to
    Makefile.frag or something like that). During the configuration you would
    just get a warning. But that's a bit off topic and just an idea that could
    be considered only for new major version... :)

    Jakub
  • Johannes Schlüter at May 28, 2014 at 7:14 pm

    On Wed, 2014-05-28 at 19:39 +0100, Jakub Zelenka wrote:

    That would make sense if we didn't have re2c generated files in the repo
    already. Anyway I see your point. Bison version varies across the systems
    more than re2c so the diffs in files would be more often which is not a
    good idea...
    The main reason is that re2c is extremely uncommon whereas flex&bison
    are common tools for developer meta packages from distributions. Also
    historically re2c had only been used for unserialisation which is way
    less touched than other scanners and parsers. Maybe when switching from
    flex to re2c for ini and source scanners this decision might have been
    revised (and I think there actually was a debate)

    johannes
  • Derick Rethans at May 29, 2014 at 8:28 am

    On Wed, 28 May 2014, Johannes Schlüter wrote:
    On Wed, 2014-05-28 at 19:39 +0100, Jakub Zelenka wrote:

    That would make sense if we didn't have re2c generated files in the repo
    already. Anyway I see your point. Bison version varies across the systems
    more than re2c so the diffs in files would be more often which is not a
    good idea...
    The main reason is that re2c is extremely uncommon whereas flex&bison
    are common tools for developer meta packages from distributions. Also
    historically re2c had only been used for unserialisation which is way
    less touched than other scanners and parsers. Maybe when switching from
    flex to re2c for ini and source scanners this decision might have been
    revised (and I think there actually was a debate)
    I find the generated files in GIT annoying. Almost every single time I
    compile I now have to changed files that GIT bitches about. I would be
    all for removing generated files from the repository.

    cheers,
    Derick
  • Adam Harvey at May 29, 2014 at 5:20 pm

    On 29 May 2014 01:28, Derick Rethans wrote:
    I find the generated files in GIT annoying. Almost every single time I
    compile I now have to changed files that GIT bitches about. I would be
    all for removing generated files from the repository.
    +1. I don't think re2c is that onerous a requirement anyway, for the
    most part: it's available through apt-get, brew, yum, and probably
    most other packaging systems. Given the amount of other things a
    developer has to install to build php-src from git, re2c is hardly
    going to break the camel's back.

    Adam
  • Nikita Popov at May 28, 2014 at 3:06 pm

    On Wed, May 28, 2014 at 1:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.

    cheers,
    Derick
    The patch (as proposed) changes the ABI for zendlex and zendparse. As such
    I don't think it's applicable to PHP 5.5 in this form.

    Nikita
  • Adam Harvey at May 29, 2014 at 5:21 pm

    On 28 May 2014 08:06, Nikita Popov wrote:
    On Wed, May 28, 2014 at 1:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.
    The patch (as proposed) changes the ABI for zendlex and zendparse. As such
    I don't think it's applicable to PHP 5.5 in this form.
    Agreed, but as Derick said, there's definitely an argument for some
    sort of Bison 3 compatibility in 5.5.

    John, can you see a way — however hackish — to get Bison 3 support in
    5.5 without changing those functions?

    Thanks,

    Adam
  • Levi Morrison at May 29, 2014 at 5:41 pm

    On Thu, May 29, 2014 at 11:21 AM, Adam Harvey wrote:
    On 28 May 2014 08:06, Nikita Popov wrote:
    On Wed, May 28, 2014 at 1:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.
    The patch (as proposed) changes the ABI for zendlex and zendparse. As such
    I don't think it's applicable to PHP 5.5 in this form.
    Agreed, but as Derick said, there's definitely an argument for some
    sort of Bison 3 compatibility in 5.5.
    My $0.02: don't touch PHP 5.5. I think it is reasonable to expect
    people to install a new version of a piece of software if their OS
    doesn't ship it by default. Mac OS X users have to do this all the
    time anyway.
  • John Bafford at May 29, 2014 at 6:01 pm

    On May 29, 2014, at 13:21, Adam Harvey wrote:
    On 28 May 2014 08:06, Nikita Popov wrote:
    On Wed, May 28, 2014 at 1:41 PM, Derick Rethans wrote:
    On Fri, 23 May 2014, Julien Pauli wrote:

    That's a no-go for 5.5.
    5.5 is stable, 1year old (at this email date), and we should not
    change such a thing into this stable branch.
    Right, but it's going to be supported for at least another year. And
    right now, on the latest Ubuntu's with only bison 3 you can't actually
    build it. Because of that, a patch like this should absolutely be
    considered.
    The patch (as proposed) changes the ABI for zendlex and zendparse. As such
    I don't think it's applicable to PHP 5.5 in this form.
    Agreed, but as Derick said, there's definitely an argument for some
    sort of Bison 3 compatibility in 5.5.

    John, can you see a way — however hackish — to get Bison 3 support in
    5.5 without changing those functions?

    Thanks,

    Adam
    If we want to fix this up for PHP 5.5, I think I see a way, and it’ll be super-hackish, but I think I can make it work. Basically, where necessary, the six functions affected (zendlex, zendparse, zenderror, ini_lex, ini_parse, and ini_error) would be given new names, and the old names would be wrapper functions with the old ABI that call-through (and either pass in NULL or TSRM_LS as appropriate). PHP internally would call the new names; the only names/wrapper functions would be there for ABI compatibility only.

    If that sounds reasonable, I’ll go ahead and give that a try, and if it works, we can merge that implementation into 5.5 and 5.6, and then I can revert the really hacky bits in the 5.6 branch since they’re not necessary because we can change the ABI.


    Two notes:

    At the least, I need to update the 5.6 version of the PR to account for that zenderror is exported as phperror, and the definition in main/php.h is wrong. (I think; following the macros is a bit confusing.) I’ll take care of that later tonight.

    I’d also suggest that perhaps in >= 5.6, those functions shouldn’t even be exported as part of the public API. In php-src, they’re basically treated as internal implementation details. zenderror and zendlex are only ever called directly from the parser, and zendparse is only ever called directly from the compile_(file|filename) functions in the scanner. (That doesn’t mean they aren’t called from external modules, but there’s a lot of setup and teardown you have to do besides, so it’d certainly be a quite advanced use. Does anyone know if those functions actually are used by pecl or other third-party modules?)

    -John
  • Stas Malyshev at May 29, 2014 at 6:12 pm
    Hi!
    I’d also suggest that perhaps in >= 5.6, those functions shouldn’t
    even be exported as part of the public API. In php-src, they’re
    basically treated as internal implementation details. zenderror and
    zendlex are only ever called directly from the parser, and zendparse
    is only ever called directly from the compile_(file|filename)
    functions in the scanner. (That doesn’t mean they aren’t called from
    external modules, but there’s a lot of setup and teardown you have to
    do besides, so it’d certainly be a quite advanced use. Does anyone
    know if those functions actually are used by pecl or other
    third-party modules?)
    I think this is reasonable, though making them inaccessible to modules
    compiled with PHP would not be possible, I think. But none of those seem
    to be suited for external consumption, so I think it's OK to make them
    more private.
    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedMay 23, '14 at 1:02a
activeMay 29, '14 at 6:12p
posts26
users12
websitephp.net

People

Translate

site design / logo © 2022 Grokbase