Contributing to PHP core means changing the actual C source code that runs on tens of millions of servers — and it is far more approachable than most PHP developers assume. This post is a complete, reproducible walkthrough of how I found, fixed, tested, and merged PR #22694 into php-src, landing in PHP 8.5. The bug: ReflectionConstant::__toString() and ReflectionClassConstant::__toString() emitted a spurious "NAN value was coerced to string" warning. The fix was four lines of C.
TL;DR#
- The bug: Casting a
NANorINFclass constant to a string via Reflection printed a warning to user code, even though the Reflection extension itself did the conversion internally. - The fix: Add an explicit
IS_DOUBLEbranch that callssmart_str_append_double()instead of falling through to a generic string coercion. - The result: 34 lines added, 0 removed, 1 test file, merged into branch
PHP-8.5. - The takeaway: Contributing to PHP core is a normal GitHub workflow — fork, branch, add a
.phpttest, open a pull request.
What bug did this PHP core contribution fix?#
PHP represents the special floating-point values NAN (Not-a-Number) and INF (Infinity) as doubles. When you cast one of these to a string in userland, PHP intentionally warns you, because silently turning NAN into the text "NAN" is usually a mistake in your own code.
The problem is that the Reflection extension performs that conversion on your behalf when you print a constant. Consider this snippet:
<?php
echo new ReflectionConstant('NAN');
class Demo {
public const MY_NAN = NAN;
}
echo new ReflectionClassConstant(Demo::class, 'MY_NAN');
Before the fix, running this on PHP 8.5 produced a warning: "unexpected NAN value was coerced to string" — even though the developer never asked for a string cast. The reflection object did. Emitting a warning for an internal conversion the user did not request is a leaky abstraction, which is exactly why this was accepted as a genuine bug (GH-22683).
What was the root cause in the C source?#
Inside ext/reflection/php_reflection.c, two helper functions build the string representation of a constant: _const_string() and _class_const_string(). Both switched over the constant's value type — IS_STRING, IS_ARRAY, and so on — but had no explicit branch for IS_DOUBLE. Double-typed constants fell through to a generic conversion path that ran the same warning-emitting coercion userland does.
Because NAN and INF are doubles, they hit that fall-through path and triggered the warning.
What is the exact fix?#
The fix adds a dedicated IS_DOUBLE branch to both functions, using PHP's internal smart_str_append_double() helper to format the value directly — the same routine PHP uses for reliable, warning-free double formatting:
else if (Z_TYPE_P(value) == IS_DOUBLE) {
smart_str_append_double(
str, Z_DVAL_P(value), (int) EG(precision), false
);
}
The trailing false argument tells the helper not to add quotes. Since smart_str_append_double() formats the double itself, the special NAN and INF values render correctly as NAN and INF with no coercion warning ever reaching user code.
Before vs. after#
| Input constant | Before the PR | After the PR |
|---|---|---|
NAN | Warning + { NAN } | { NAN }, no warning |
INF | Warning + { INF } | { INF }, no warning |
1.5 | { 1.5 } | { 1.5 } |
2.0 | { 2 } | { 2 } |
How is the fix tested?#
Every behavioral change to PHP core needs a test in the .phpt format. This PR added ext/reflection/tests/gh22683.phpt, which reflects over NAN, INF, and ordinary floats through both ReflectionConstant and ReflectionClassConstant, then asserts the output contains no warnings and formats as { NAN }, { INF }, { 1.5 }, and { 2 }. A .phpt file pins the exact expected output, so any future regression fails the CI suite immediately.
How do you start contributing to PHP core yourself?#
Here is the exact workflow this PR followed. If you have ever opened a GitHub pull request, none of this is new — the only unusual part is that the codebase is C, not PHP.
- Find a real issue. Browse the php-src issue tracker for bugs labeled as confirmed or good-first-bug. A small, well-scoped bug like a spurious warning is ideal for a first PHP core contribution.
- Fork and build. Fork
php/php-src, then./buildconf && ./configure && make. Building PHP from source locally is the single biggest hurdle — once it compiles, iteration is fast. - Locate the code. Extensions live under
ext/. Reflection isext/reflection/php_reflection.c. Grep for the warning text or the function name to find the exact spot. - Write the fix and a
.phpttest. Keep the change minimal and match surrounding conventions. - Run
make test. Confirm your new test passes and nothing else breaks. - Open the pull request against the right branch. Bug fixes target the lowest supported branch that has the bug — here,
PHP-8.5. Reference the issue number (GH-22683) in the title. - Respond to review. Core maintainers review quickly when the change is small, tested, and clearly scoped.
This is my second merged Reflection patch — the first added ReflectionConstant::inNamespace(). If you want the full story of that one, read how PR #20902 was merged into PHP core. And if you enjoy language-design rabbit holes, see my RFC idea for loop unrolling via a #[Unroll(N)] attribute.
Why does a four-line PHP core fix matter?#
Small correctness fixes compound. A spurious warning forces developers to litter code with @ error suppression or custom error handlers, both of which hide real problems. Removing the warning at the source means Reflection-based tooling — documentation generators, static analyzers, debuggers, and IDE plugins that introspect constants — can print float constants cleanly. That is the quiet value of contributing to PHP core: one precise change improves every tool built on top of the language.
Frequently asked questions#
Do I need to know C to contribute to PHP core?#
You need enough C to read the surrounding code and make a focused change. Many first contributions — like this NAN fix — are a handful of lines. You do not need to be a systems programmer; you need to be careful and to follow existing patterns.
What is a .phpt file?#
A .phpt file is PHP's native test format. It contains sections such as --TEST--, --FILE-- (the PHP code to run), and --EXPECT-- (the exact expected output). Running make test executes them and diffs actual against expected output.
Which branch should a bug-fix PR target?#
Bug fixes target the oldest still-supported branch affected by the bug, so the fix can be merged up into newer branches. New features target the current development branch. This PR targeted PHP-8.5.
How long does it take to get a PHP core PR merged?#
It depends on scope and reviewer availability, but small, well-tested, clearly-scoped fixes are often reviewed and merged within days. A minimal diff with a passing test is the fastest path.
Where can I find good first bugs in php-src?#
Start with the php-src GitHub issues and the official Reflection documentation to understand expected behavior, then look for confirmed bugs with a clear reproduction case.
Key takeaways#
- Contributing to PHP core uses an ordinary fork-branch-PR workflow; the codebase is C.
- The bug was a leaky abstraction: Reflection warned about a conversion it performed itself.
- The fix adds an explicit
IS_DOUBLEbranch callingsmart_str_append_double(). - Every behavioral change ships with a
.phpttest. - Start small: a spurious-warning fix is an ideal first contribution.
Have you contributed to PHP core, or are you eyeing your first patch? I'm @khaledalam on GitHub — I'm happy to point you at a good first bug.
Comments
No comments yet — be the first.