Simplify console output helpers - #630
Conversation
|
Supersedes #629 (the minimal |
Replace the APC_PRINT_FUNCTION macro that generated four exported apc_error/apc_warning/apc_notice/apc_debug functions with thin variadic macros in apc.h that forward directly to php_error_docref(), and drop the generator block from apc.c entirely. This also fixes the build against PHP 8.6-dev: php/php-src@0a12b3e8268 removed the params argument from php_verror(), which the old APC_PRINT_FUNCTION body called. Forwarding to php_error_docref() instead sidesteps php_verror() altogether and works on all supported versions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
22aaf7a to
b457c69
Compare
|
Pushed a small refinement: with do { if (0) php_error_docref(NULL, E_NOTICE, __VA_ARGS__); } while (0)instead of a bare |
madmajestro
left a comment
There was a problem hiding this comment.
LGTM! Thank you!
I think this ABI break is more of a theoretical issue. No other PHP extension should rely on APCu logging functions that were quite obviously intended for internal use. Especially since that functionality is also available via php_error_docref() in the PHP core. Not even the apcu-bc extension depends on these functions.
|
Thank you, awaiting beta stability of 8.6 now |
Simplify console output helpers
Follow-up to #629, per @madmajestro's review suggestion there.
Replaces the
APC_PRINT_FUNCTIONgenerator inapc.c— which produced fourexported
apc_error/apc_warning/apc_notice/apc_debugfunctions —with thin variadic macros in
apc.hthat forward straight tophp_error_docref():Net −16 lines, and it supersedes #629: the old generator body called
php_verror(), whose signature changed in php/php-src@0a12b3e8268 (params argremoved) and broke the build against PHP 8.6-dev. Forwarding to
php_error_docref()sidestepsphp_verror()entirely and builds on allsupported versions. If this lands, #629 can be closed.
Trade-off: ABI
Pro — simpler (no
apc.ccode, nova_list/vspprintf/efreeplumbing), one fewer indirection, and format-string checking is preserved via
php_error_docref's ownprintfattribute.apc_debugalso stops evaluatingits arguments when
APC_DEBUGis off (do {} while (0)), which is the usual,slightly-better behaviour.
Con — the four functions were declared
PHP_APCU_API, i.e. exportedsymbols; turning them into macros removes them from the ABI. They're internal
console helpers (no in-tree or
bc/caller takes their address or uses themoutside a direct call), so the practical risk is nil — but it is technically a
public-symbol removal, hence a separate PR for you to decide rather than folded
into the #629 build-fix.
Verification
master(new 4-argphp_verrorpresent inmain/php.h).masterapcu fails the same compile on thephp_verrorargument mismatch — the exact break Fix build against PHP 8.6 (php_verror params argument removed) #629 addresses — confirmingthe fix is real and not a no-op.
APC_DEBUG=1build compiles; no-Wunusedfallout from the disabledapc_debug.