Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:98807
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.20.1704151624210.2260@singlemalt.home.derickrethans.nl>
References: <alpine.DEB.2.20.1704151624210.2260@singlemalt.home.derickrethans.nl>
Date: Sat, 15 Apr 2017 18:24:53 +0200
Message-ID: <CAF+90c8ExHwbtj250UotjYu4mCwvj0yGBr-j-z4mv5_1N3dcow@mail.gmail.com>
To: Derick Rethans <derick@php.net>
Cc: PHP Developers Mailing List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=94eb2c0685ce7ee054054d36ffb9
Subject: Re: Inconsistency in line numbers.
From: nikita.ppv@gmail.com (Nikita Popov)

--94eb2c0685ce7ee054054d36ffb9
Content-Type: text/plain; charset=UTF-8

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/183cd048f18fa4b04fb30448a84a54cee80a2491,
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/e433c23b96e81340cd0e2d0b4b7f5fce7f72a931
.

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

--94eb2c0685ce7ee054054d36ffb9--