On Sat, Apr 15, 2017 at 1:13 PM, Derick Rethans <der...@php.net> wrote:
> On April 15, 2017 5:24:53 PM GMT+01:00, Nikita Popov <nikita....@gmail.com> > wrote: > >On Sat, Apr 15, 2017 at 5:25 PM, Derick Rethans <der...@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?
