Re: Bison 3 support for PHP 5.5

Pierre, On May 23, 2014, at 01:40, Pierre Joye <pierre.php@gmail.com> wrote: > Hi John, > > On Fri, May 23, 2014 at 3:44 AM, John Bafford <john@bafford.com> wrote: >> Hi Rasmus, >> >> On May 22, 2014, at 20:28, Rasmus Lerdorf <rasmus@lerdorf.com> 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