Req #80835 [Opn->Csd]: suggested code cleanup for ZEND_ENABLE_STATIC_TSRMLS_CACHE

Edit report at https://bugs.php.net/bug.php?id=80835&edit=1 ID: 80835 Updated by: cmb@php.net Reported by: theultramage at gmail dot com Summary: suggested code cleanup for ZEND_ENABLE_STATIC_TSRMLS_CACHE -Status: Open +Status: Closed Type: Feature/Change Request -Package: Unknown/Other Function +Package: Scripting Engine problem Operating System: FreeBSD 12.2 PHP Version: 8.0.3 -Assigned To: +Assigned To: cmb Block user comment: N Private report: N New Comment: Since we don't have control over external extensions, setting the flag for each extension is likely necessary. The additional wrappers are likely for loose coupling (Zend/* should ideally not rely on TSRM/*). Anyhow, I don't think it is helpful to keep this ticket open. Feel free to provide pull requests[1] for code cleanup, instead. [1] <https://github.com/php/php-src/pulls> Previous Comments: ------------------------------------------------------------------------ [2021-03-05 14:11:39] theultramage at gmail dot com Description: ------------ In 2014, a bunch of defines for the experimental TSRM cache were added to zend.h, and across multiple commits, they were applied to the base code and extensions code, as well as their build scripts. https://github.com/php/php-src/commit/76081df168829a5cc0409fac47c217d4927ec6f6 https://github.com/php/php-src/commit/5749b4a9979cd3ff85996323bed9adc1bd182f76 In particular, the ZEND_ENABLE_STATIC_TSRMLS_CACHE define acts as an on/off toggle. And for some reason, the developer decided to add the compiler flag ZEND_ENABLE_STATIC_TSRMLS_CACHE=1 into every possible m4 makefile. I have questions. - Was it necessary to add this level of control over that flag, making a mess in the process? I now suspect that the sole reason it's there is because the developer was going through the extensions one by one, and only wanted to turn the cache defines on after they made the required code edits. In that case, this shim should have been removed before the development branch was merged. - Was it necessary to add a set of wrapper defines to zend.h that just point to tsrm.h? Couldn't this have been contained within tsrm.h, to avoid having to rebrand the TSRM api as ZEND_TSRM through a bunch of cosmetic src edits? All the extensions still #include "TSRM.h", and 18 of those don't even use TSRM. - In several places the work wasn't done, and those still directly use the non-cache TSRM macros. In some others, the code directly calls cache-specific TSRM macros, leading to a compilation error if the cache is disabled. See https://bugs.php.net/bug.php?id=80823 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=80835&edit=1