Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:98809
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.54 as permitted sender)
MIME-Version: 1.0
Sender: morrison.levi@gmail.com
In-Reply-To: <797C79DF-B6A8-4012-8584-26CF816A1278@php.net>
References: <alpine.DEB.2.20.1704151624210.2260@singlemalt.home.derickrethans.nl>
 <CAF+90c8ExHwbtj250UotjYu4mCwvj0yGBr-j-z4mv5_1N3dcow@mail.gmail.com> <797C79DF-B6A8-4012-8584-26CF816A1278@php.net>
Date: Sat, 15 Apr 2017 15:14:06 -0600
Message-ID: <CAFMT4No3XR_=yq+shy+GXCPS+CsoWXGk4UsL6WLocBY8vbc3fA@mail.gmail.com>
To: Derick Rethans <derick@php.net>
Cc: Nikita Popov <nikita.ppv@gmail.com>, 
	PHP Developers Mailing List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1144820ed6abee054d3b0987
Subject: Re: [PHP-DEV] Re: Inconsistency in line numbers.
From: levim@php.net (Levi Morrison)

--001a1144820ed6abee054d3b0987
Content-Type: text/plain; charset=UTF-8

On Sat, Apr 15, 2017 at 1:13 PM, Derick Rethans <derick@php.net> wrote:

> On April 15, 2017 5:24:53 PM GMT+01:00, Nikita Popov <nikita.ppv@gmail.com>
> wrote:
> >On Sat, Apr 15, 2017 at 5:25 PM, Derick Rethans <derick@php.net> wrote:
> >
> >> Hi Nikita,
> >>
> >> Through Xdebug bug #1413 (https://bugs.xdebug.org/view.php?id=1413),
> >I
> >> noticed
> >> that behaviour with class constants changed between PHP 7.1.3 and PHP
> >> 7.1.4.
> >> I *believe* it is because of the fix for issue #69676
> >> (https://bugs.php.net/bug.php?id=69676) that got fixed in PHP 7.1.4.
> >>
> >> With OPcache loaded, the INIT_ARRAY and further opcodes are relisted
> >as
> >> being
> >> on line 15 instead of line 17. Although, I would probably argue that
> >> INIT_ARRAY should really be on line 13 (the opening [).
> >>
> >> Without OPcache loaded, in PHP 7.1.3, FETCH_CLASS_CONSTANT is on line
> >15,
> >> and
> >> the INIT_ARRAY and further opcodes are on line 17. In PHP 7.1.4, they
> >are
> >> all
> >> on line 15 - with 17 no longer showing up.
> >>
> >> I believe, the behaviour with and without OPcache should be the same,
> >and I
> >> would argue that the correct result would need to be that
> >> FETCH_CLASS_CONSTANT
> >> (if still present, OPcache optimises it out) should be on line 15,
> >the
> >> INIT_ARRAY on line 13, and the SEND_VAL_EX and further, are on line
> >17.
> >>
> >> Code
> >> ====
> >>
> >>   1 <?php
> >>   2
> >>   3 namespace ICanBoogie\Validate\Validator;
> >>   4
> >>   5 class Required2
> >>   6 {
> >>   7     const ALIAS = 'required';
> >>   8     const DEFAULT_MESSAGE = "is required";
> >>   9     const OPTION_STOP_ON_ERROR = 'stop_on_error';
> >>  10
> >>  11     public function normalize_params(array $params)
> >>  12     {
> >>  13         return array_merge([
> >>  14
> >>  15             self::OPTION_STOP_ON_ERROR => true
> >>  16
> >>  17         ], $params);
> >>  18     }
> >>  19 }
> >>
> >>
> >> PHP 7.1.3 (no OPcache)
> >> ======================
> >>
> >> filename:       /tmp/xdebug-bug-1/lib/Required2.php
> >> function name:  normalize_params
> >> number of ops:  14
> >> compiled vars:  !0 = $params
> >> line     #* E I O op                           fetch          ext
> >return
> >> operands
> >> ------------------------------------------------------------
> >> -------------------------
> >>   11     0  E >   EXT_NOP
> >>          1        RECV                                             !0
> >>   13     2        EXT_STMT
> >>          3        INIT_NS_FCALL_BY_NAME
> >>          4        EXT_FCALL_BEGIN
> >>   15     5        FETCH_CLASS_CONSTANT                             ~1
> >> 'OPTION_STOP_ON_ERROR'
> >>   17     6        INIT_ARRAY                                       ~2
> >> <true>, ~1
> >>          7        SEND_VAL_EX
> >> ~2
> >>          8        SEND_VAR_EX
> >> !0
> >>          9        DO_FCALL_BY_NAME
> >>         10        EXT_FCALL_END
> >>         11      > RETURN
> >>  $3
> >>   18    12*       EXT_STMT
> >>         13*     > RETURN
> >>  null
> >>
> >>
> >> PHP 7.1.3 (OPcache)
> >> ======================
> >>
> >> filename:       /tmp/xdebug-bug-1/lib/Required2.php
> >> function name:  normalize_params
> >> number of ops:  11
> >> compiled vars:  !0 = $params
> >> line     #* E I O op                           fetch          ext
> >return
> >> operands
> >> ------------------------------------------------------------
> >> -------------------------
> >>   11     0  E >   EXT_NOP
> >>          1        RECV                                             !0
> >>   13     2        EXT_STMT
> >>          3        INIT_NS_FCALL_BY_NAME
> >>          4        EXT_FCALL_BEGIN
> >>   17     5        INIT_ARRAY                                       ~1
> >> <true>, 'stop_on_error'
> >>          6        SEND_VAL_EX
> >> ~1
> >>          7        SEND_VAR_EX
> >> !0
> >>          8        DO_FCALL                                      0  $1
> >>          9        EXT_FCALL_END
> >>         10      > RETURN
> >>  $1
> >>
> >>
> >> PHP 7.1.4 (no OPcache)
> >> ======================
> >>
> >> filename:       /tmp/xdebug-bug-1/lib/Required2.php
> >> function name:  normalize_params
> >> number of ops:  14
> >> compiled vars:  !0 = $params
> >> line     #* E I O op                           fetch          ext
> >return
> >> operands
> >> ------------------------------------------------------------
> >> -------------------------
> >>   11     0  E >   EXT_NOP
> >>          1        RECV                                             !0
> >>   13     2        EXT_STMT
> >>          3        INIT_NS_FCALL_BY_NAME
> >>          4        EXT_FCALL_BEGIN
> >>   15     5        FETCH_CLASS_CONSTANT                             ~1
> >> 'OPTION_STOP_ON_ERROR'
> >>          6        INIT_ARRAY                                       ~2
> >> <true>, ~1
> >>          7        SEND_VAL_EX
> >> ~2
> >>          8        SEND_VAR_EX
> >> !0
> >>          9        DO_FCALL_BY_NAME
> >>         10        EXT_FCALL_END
> >>         11      > RETURN
> >>  $3
> >>   18    12*       EXT_STMT
> >>         13*     > RETURN
> >>  null
> >>
> >>
> >> PHP 7.1.4 (OPcache)
> >> ======================
> >>
> >> filename:       /tmp/xdebug-bug-1/lib/Required2.php
> >> function name:  normalize_params
> >> number of ops:  11
> >> compiled vars:  !0 = $params
> >> line     #* E I O op                           fetch          ext
> >return
> >> operands
> >> ------------------------------------------------------------
> >> -------------------------
> >>   11     0  E >   EXT_NOP
> >>          1        RECV                                             !0
> >>   13     2        EXT_STMT
> >>          3        INIT_NS_FCALL_BY_NAME
> >>          4        EXT_FCALL_BEGIN
> >>   15     5        INIT_ARRAY                                       ~1
> >> <true>, 'stop_on_error'
> >>          6        SEND_VAL_EX
> >> ~1
> >>          7        SEND_VAR_EX
> >> !0
> >>          8        DO_FCALL                                      0  $1
> >>          9        EXT_FCALL_END
> >>         10      > RETURN
> >>  $1
> >>
> >> cheers,
> >> Derick
> >>
> >
> >This is due to
> >https://github.com/php/php-src/commit/183cd048f18fa4b04fb30448a84a54
> cee80a2491,
> >which fixed lineno assignment for list AST nodes. This changed the
> >start
> >lineno of the array from line 17 (the end of the array) to line 15 (the
> >first array element). Of course this is still inaccurate (the actual
> >start
> >line is 13), but it's the closest we can get with the information we
> >currently have available.
> >
> >The problem in this case is that the second SEND_VAR_EX did not bump
> >the
> >lineno. I've fixed this in
> >https://github.com/php/php-src/commit/e433c23b96e81340cd0e2d0b4b7f5f
> ce7f72a931
> >.
> >
> >This required a change to an existing test, which alerted me to some
> >weird
> >behavior I wasn't aware of: https://3v4l.org/6fSRb In backtraces the
> >call
> >is located on the line of it's last argument (in PHP 5.6 the line of
> >the
> >closing ")"). Wouldn't it make more sense to use the starting line of
> >the
> >call? That is, use the same lineno for INIT_FCALL and DO_FCALL?
> >
> >Nikita
>
> It makes sense to me to use the first line, as long as further opcodes
> will have the right line  number.
>

I agree the first line makes sense.


> Selfishly, I'd prefer if that change was only made for master, due to the
> increasingly complex tests I need to make for Xdebug's code coverage.


That means it will go into 7.2 and not into any 7.1 fix, correct?

--001a1144820ed6abee054d3b0987--