Repository navigation
php: fix upstream's typo in the no-atomics arm, which nobody compiles - #726
Merged
Merged
Conversation
zend_atomic.c:44 answered `argument prototype mismatch "IND STRUCT zend_atomic_bool_s" for "IND STRUCT zend_atomic_int_s"'. zend_atomic.h:323 declares zend_atomic_bool_compare_exchange_ex(zend_atomic_int *obj, ...) while the other four arms -- C11, GNUC, SYNC and WIN32 at :106, :161, :207 and :263 -- all say zend_atomic_bool *, as does every sibling _bool_ function in that same arm, and zend_atomic.c passes a zend_atomic_bool *. One word, fixed in the vendored source with the reasoning beside it. It survives because nobody compiles that arm: it is reached only by a platform with no C11 atomics, no GCC atomics, no __sync builtins and not Windows, which until kencc was no platform at all. And gcc only warns where kencc errors, so even someone who did compile it might not have noticed. Worth reporting upstream. The arm has exactly one bug, and that is measured rather than hoped. Compiling it in isolation found nothing, because the mismatch is between a caller and a callee and the arm is self-consistent within itself; forcing HAVE_NO_ATOMICS in a copy of the header and compiling the real zend_atomic.c against it reproduces the kencc diagnostic exactly, reports nothing else, and after the fix reports nothing at all. Forcing it took two attempts, which is the part worth keeping. The first -I did nothing, because zend_atomic.c includes "zend_atomic.h" quoted and that resolves next to the source file before any -I; the compile silently took the GNUC arm and reported a clean result. A forced configuration that was not actually forced reads exactly like a passing one, and the tell was the absent #warning. Also drops a `linux/*' from a comment in php_config.h, which put a `"/*" within comment' warning into all 513 compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
zend_atomic.c:44 answered `argument prototype mismatch "IND STRUCT zend_atomic_bool_s" for "IND STRUCT zend_atomic_int_s"'. zend_atomic.h:323 declares
zend_atomic_bool_compare_exchange_ex(zend_atomic_int *obj, ...) while the other four arms -- C11, GNUC, SYNC and WIN32 at :106, :161, :207 and :263 -- all say zend_atomic_bool *, as does every sibling bool function in that same arm, and zend_atomic.c passes a zend_atomic_bool *. One word, fixed in the vendored source with the reasoning beside it.
It survives because nobody compiles that arm: it is reached only by a platform with no C11 atomics, no GCC atomics, no __sync builtins and not Windows, which until kencc was no platform at all. And gcc only warns where kencc errors, so even someone who did compile it might not have noticed. Worth reporting upstream.
The arm has exactly one bug, and that is measured rather than hoped. Compiling it in isolation found nothing, because the mismatch is between a caller and a callee and the arm is self-consistent within itself; forcing HAVE_NO_ATOMICS in a copy of the header and compiling the real zend_atomic.c against it reproduces the kencc diagnostic exactly, reports nothing else, and after the fix reports nothing at all.
Forcing it took two attempts, which is the part worth keeping. The first -I did nothing, because zend_atomic.c includes "zend_atomic.h" quoted and that resolves next to the source file before any -I; the compile silently took the GNUC arm and reported a clean result. A forced configuration that was not actually forced reads exactly like a passing one, and the tell was the absent #warning.
Also drops a
linux/*' from a comment in php_config.h, which put a"/*" within comment' warning into all 513 compiles.Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs