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?