Hi Rowan Tommins,
> I would like to propose we formally deprecate the function debug_zval_dump and remove it in PHP 9.0.
>
> 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 interpret, but is now so complex as to be effectively useless:
>
> - Because it is implemented as a function taking a parameter in the normal way, the refcount has been modified by the time it is displayed. Depending on the value passed, this may include reference separation; in older versions of PHP, it was possible to affect this directly by forcing a pass-by-reference. The manual still discusses this, but it hasn't been possible since PHP 5.4. [1]
> - Since PHP 7, some types don't have a refcount at all, and references are represented by additional levels of zval. Without completely changing the output format, this information is impossible to convey accurately.
> - Optimisations affect the refcount in increasingly non-obvious ways. For instance, an array defined literally in the source code has an extra counted reference compared to one which has been modified at runtime. [2]
>
> Since this is a rather specialised piece of debugging information, not useful 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 it up in the symbol table, avoiding some of the worst issues [3]. I'm not familiar with PHPDBG, and it doesn't seem to have much documentation, but I assume it would be able to display this as well.
I'd disagree that it's useless. Even with the format changes,
- Checking the difference between two runs is useful in a bug report - if counts increase or decrease after calling a function
that isn't supposed to modify reference counts, this makes it an obvious indicator for reference counting bugs that can be submitted
or requested on issue trackers such as bugs.php.net
- Dynamic values (e.g. db results) generally aren't constant values, with some exceptions.
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 that average users use every day.**
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,
and having debug_zval_dump is useful for investigating, detecting, reporting, or adding regression tests for reference counting bugs.
- Instructions often recommend that users/packagers run tests before installing a PECL.
If running a subset of the tests required a third party PECL, those tests might just end up being skipped
and memory counting bugs in new PHP versions or rare PHP configurations (32-bit ZTS) might be harder to track down,
or PECL authors may just not get around to installing and enabling external PECLs in CI.
- A maintainer or someone looking at a bug report on bugs.php.net may be reluctant (or not have the time) to install and enable a PECL
they're unfamiliar with in order to check if a bug report was reproducible, making bugs take longer to fix or never get fixed.
Even if the debug_zval_dump doesn't end up in the final phpt test case or the final PR created to fix a bug in PECLs,
it may have been used while tracking down which function had the reference counting bug.
Many of the C functions in php-src have no documentation whatsoever, so it's hard for new and experienced developers
to tell if they do/don't need to addref/delref before/after calling a function because surrounding code (in code they're basing new code on)
may have adjusted the reference count in other ways already.
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,
e.g. if the code was prone to bugs and they wanted to assert the reference count or use of references was correct.
(usage might be limited by the obscurity and not knowing about it - having PECL writing tutorials mention it exists for tracking down
reference counting bugs may make it more widely used)
- https://github.com/runkit7/runkit7/blob/master/tests/runkit_superglobals_obj_alias.phpt#L111
- https://github.com/krakjoe/apcu/blob/master/tests/apc_006.phpt
- https://github.com/php/php-src/search?q=debug_zval_dump&type=code
- https://bugs.php.net/search.php?search_for=debug_zval_dump&boolean=0&limit=100&cmd=display&status=All&bug_type=All
https://github.com/runkit7/runkit7/blob/master/tests/runkit_superglobals_obj_alias.phpt#L30
Also, the last time I checked, XDebug replaces the php interpreter entirely and is slower than the php interpreter,
so I wouldn't consider it a viable replacement (especially if a bug only manifests with the standard php interpreter).
> 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?
My personal preference would be a separate RFC because there's no available replacement.
- At a glance, all of the functionality in the omnibus have a replacement suggested that's in php-src or the same module.
debug_zval_dump doesn't, and someone unfamiliar with `debug_zval_dump` might assume it does have a replacement
because everything else in the omnibus did have a replacement.
This also seems like it's being proposed out of a desire to have fewer print functions rather than a problem with the implementation of debug_zval_dump
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.
Moving it into a new optional module to which more debugging functionality might be added in separate RFCs might be an alternative (to deprecation)
satisfying some of the same goals, but I still don't see a reason to remove it.
Regards,
Tyson
