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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to