Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113316
To: "internals@lists.php.net" <internals@lists.php.net>
Thread-Topic: [PHP-DEV] Deprecate debug_zval_dump?
Thread-Index: AQHXDRDmxKFMX8CoyEKjbU51D3tYq6psVy7I
Date: Sat, 27 Feb 2021 19:29:57 +0000
Message-ID:
 <DM6PR07MB66189C81A05409F9FFFA8A49F99C9@DM6PR07MB6618.namprd07.prod.outlook.com>
References: <62243527-96EC-4234-AD50-EDB6333628D5@gmail.com>
In-Reply-To: <62243527-96EC-4234-AD50-EDB6333628D5@gmail.com>
Accept-Language: en-CA, en-US
Content-Language: en-CA
x-ms-exchange-messagesentrepresentingtype: 1
x-tmn: [9quQm7BKWKD8WKcrrA0whkuJ2eGXtVKl8Chled0fX4n1oZki3Deb4VbG+E8D8sr8]
x-ms-publictraffictype: Email
x-incomingheadercount: 44
x-eopattributedmessage: 0
x-ms-office365-filtering-correlation-id: f11a86b9-6b0b-441b-0877-08d8db5610fa
x-ms-traffictypediagnostic: CO1NAM04HT208:
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 b7tCRPaOs6YK0VpNDDYgmecpPiByrOGoVgXcW+zG584EZHO/FFldGezFkB21kYOUaeocECWlo13lRu557PfF5aOWJM6FWOS7O/dxhCcg2fqXrEFyLZLHWj29TpVgiwi3PCKNwTmqg7UAxHjiTQK7LusUwmcQyxdfBVYjsvEbUYT8fbkUgZuu+xposuZ2KkE4XqwNJFDq7Q83MWVekpTr1GfI8k6g/3wgnMG4wRmaxqVOz3XNzbntJu8Bbq7ZeTfTejpo+QIe9DnsgAOlIjlYVB6d0/E06l3JRbg4pkmzFnyXiknnnp0AjCblYUQRK0ez4E7LineszWDU2E7q6wkvqZ9uamvJGi121yqOcegv/+21BADySZPyMF0PBAiFS1zWT8t0a8YanLqXKDfdbpggt44DV5+xQxiK+jZw//jzSpUEB54lEmzIKeePsDMxM5+/
x-ms-exchange-antispam-messagedata:
 P4p4UMyBh/VRjP2voQEiUPPXbPGFy8WVlXudQn7GVZM7D8Eyrvtl3/Y4eMNXBVaE6EHHiX5zgaGwYjzTpY2jiuZoKiPHBgcjaMjG69Hwom0wR7XJjQrTJRMDcpSx+lbJqBJP2R/8HyKpMQKtJ1ufNccOh3+J3Q9pbi6sHVH9GGGeavFGq6pajI2uTwpf/rr/iMclwZPYz23VSC/K6C180Q==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: hotmail.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-AuthSource: CO1NAM04FT054.eop-NAM04.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: f11a86b9-6b0b-441b-0877-08d8db5610fa
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Feb 2021 19:29:57.0387
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Internet
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1NAM04HT208
Subject: Re: [PHP-DEV] Deprecate debug_zval_dump?
From: tysonandre775@hotmail.com (tyson andre)

Hi Rowan Tommins,=0A=
=0A=
> I would like to propose we formally deprecate the function debug_zval_dum=
p and remove it in PHP 9.0.=0A=
> =0A=
> This function is mostly similar to var_dump, with the additional ability =
to output the refcount of a variable. This has always been hard to interpre=
t, but is now so complex as to be effectively useless:=0A=
> =0A=
> - Because it is implemented as a function taking a parameter in the norma=
l way, the refcount has been modified by the time it is displayed. Dependin=
g on the value passed, this may include reference separation; in older vers=
ions of PHP, it was possible to affect this directly by forcing a pass-by-r=
eference. The manual still discusses this, but it hasn't been possible sinc=
e PHP 5.4. [1]=0A=
> - Since PHP 7, some types don't have a refcount at all, and references ar=
e represented by additional levels of zval. Without completely changing the=
 output format, this information is impossible to convey accurately.=0A=
> - Optimisations affect the refcount in increasingly non-obvious ways. For=
 instance, an array defined literally in the source code has an extra count=
ed reference compared to one which has been modified at runtime. [2]=0A=
> =0A=
> Since this is a rather specialised piece of debugging information, not us=
eful to the average user, I think it should be left to dedicated debugging =
tools. XDebug includes an equivalent function that takes a name and looks i=
t up in the symbol table, avoiding some of the worst issues [3]. I'm not fa=
miliar with PHPDBG, and it doesn't seem to have much documentation, but I a=
ssume it would be able to display this as well.=0A=
=0A=
I'd disagree that it's useless. Even with the format changes,=0A=
=0A=
- Checking the difference between two runs is useful in a bug report - if c=
ounts increase or decrease after calling a function =0A=
  that isn't supposed to modify reference counts, this makes it an obvious =
indicator for reference counting bugs that can be submitted =0A=
  or requested on issue trackers such as bugs.php.net=0A=
- Dynamic values (e.g. db results) generally aren't constant values, with s=
ome exceptions.=0A=
=0A=
I'm opposed to the removal - while it is not useful to the average user, **=
it is very useful in the development of php-src and the PECL extensions tha=
t average users use every day.** =0A=
E.g. the php developers working on php-src or contributors to PECLs may use=
 it while investigating memory reference counting bugs or while developing =
new functionality,=0A=
and having debug_zval_dump is useful for investigating, detecting, reportin=
g, or adding regression tests for reference counting bugs.=0A=
=0A=
- Instructions often recommend that users/packagers run tests before instal=
ling a PECL.=0A=
  If running a subset of the tests required a third party PECL, those tests=
 might just end up being skipped=0A=
  and memory counting bugs in new PHP versions or rare PHP configurations (=
32-bit ZTS) might be harder to track down,=0A=
  or PECL authors may just not get around to installing and enabling extern=
al PECLs in CI.=0A=
- A maintainer or someone looking at a bug report on bugs.php.net may be re=
luctant (or not have the time) to install and enable a PECL =0A=
  they're unfamiliar with in order to check if a bug report was reproducibl=
e, making bugs take longer to fix or never get fixed.=0A=
=0A=
Even if the debug_zval_dump doesn't end up in the final phpt test case or t=
he final PR created to fix a bug in PECLs,=0A=
it may have been used while tracking down which function had the reference =
counting bug.=0A=
=0A=
Many of the C functions in php-src have no documentation whatsoever, so it'=
s hard for new and experienced developers =0A=
to tell if they do/don't need to addref/delref before/after calling a funct=
ion because surrounding code (in code they're basing new code on)=0A=
may have adjusted the reference count in other ways already.=0A=
=0A=
I've seen a few places where debug_zval_dump is added in tests in order to =
ensure that the reference count didn't change,=0A=
e.g. if the code was prone to bugs and they wanted to assert the reference =
count or use of references was correct.=0A=
(usage might be limited by the obscurity and not knowing about it - having =
PECL writing tutorials mention it exists for tracking down=0A=
reference counting bugs may make it more widely used)=0A=
=0A=
- https://github.com/runkit7/runkit7/blob/master/tests/runkit_superglobals_=
obj_alias.phpt#L111=0A=
- https://github.com/krakjoe/apcu/blob/master/tests/apc_006.phpt=0A=
- https://github.com/php/php-src/search?q=3Ddebug_zval_dump&type=3Dcode=0A=
- https://bugs.php.net/search.php?search_for=3Ddebug_zval_dump&boolean=3D0&=
limit=3D100&cmd=3Ddisplay&status=3DAll&bug_type=3DAll=0A=
=0A=
https://github.com/runkit7/runkit7/blob/master/tests/runkit_superglobals_ob=
j_alias.phpt#L30=0A=
=0A=
Also, the last time I checked, XDebug replaces the php interpreter entirely=
 and is slower than the php interpreter,=0A=
so I wouldn't consider it a viable replacement (especially if a bug only ma=
nifests with the standard php interpreter).=0A=
=0A=
> I notice there's a draft for an omnibus "deprecations for PHP 8.1" RFC [4=
]. Should I add this there, or raise a separate RFC?=0A=
=0A=
My personal preference would be a separate RFC because there's no available=
 replacement.=0A=
=0A=
- At a glance, all of the functionality in the omnibus have a replacement s=
uggested that's in php-src or the same module.=0A=
  debug_zval_dump doesn't, and someone unfamiliar with `debug_zval_dump` mi=
ght assume it does have a replacement=0A=
  because everything else in the omnibus did have a replacement.=0A=
=0A=
This also seems like it's being proposed out of a desire to have fewer prin=
t functions rather than a problem with the implementation of debug_zval_dum=
p=0A=
or the existence a better alternative to the function itself - in practice,=
 end users likely aren't harmed by keeping it around because it's obviously=
 an obscure function for debugging.=0A=
=0A=
Moving it into a new optional module to which more debugging functionality =
might be added in separate RFCs might be an alternative (to deprecation)=0A=
satisfying some of the same goals, but I still don't see a reason to remove=
 it.=0A=
=0A=
Regards,=0A=
Tyson=0A=