Skip to content

Patch the 35 implicitly-nullable vendor parameters (PHP 8.4 readiness) - #24988

Merged
PurHur merged 2 commits into
masterfrom
fix/php84-vendor-implicit-nullable-patch
Jul 29, 2026
Merged

PurHur merged 2 commits into
masterfrom
fix/php84-vendor-implicit-nullable-patch

Conversation

@PurHur

@PurHur PurHur commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Completes the implicit-nullable work started in #24972.

PHP 8.4 deprecates Foo $x = null and writes the notice to stderr, which the compliance harness compares — so on an 8.4 host every case whose path touches one of these would differ from Zend on stderr alone.

35 parameters across 23 files in php-cfg and php-optimizer, via the existing patches/ layer (167 patches already there). ?Type has been valid since PHP 7.1 and these declarations already accepted null, so behaviour is unchanged on every supported version. The counterpart declarations in lib/Visitor/* were fixed directly in #24972, so interface and implementations stay consistent.

Measured

php-compiler:8.4-dev (PHP 8.4.23), bin/vm.php hello-world:

before after
implicit-nullable deprecations 16 0
total deprecations 17 1

The one remaining is a different 8.4 deprecation class entirely — Using "_" as a class name is deprecated since 8.4 in ext/gettext/_.php — not addressed here.

Verified

  • apply-patches.sh rc=0, and rc=0 again on a second run (idempotent). The three "error"-matching lines are pre-existing Skip … (already applied) messages whose patch names contain "error" — identical count on master.
  • php-compiler:22.04-dev (PHP 8.2.32) with a fresh composer install: apply-patches rc=0, vm rc=0, stdout hello 3, zero stderr.
  • Patch generated by diffing against a pristine backup, then vendor restored and confirmed byte-identical before committing — so the patch is the only thing that applies the change.

Process note

The first attempt wired the apply_patch call into a conditional helper whose anchor appears twice. The edit asserted and failed, and the follow-up measurement showed 16 deprecations — which read as the patch not working rather than the patch never having run. Wired at top level instead, alongside the other unconditional php-llvm patches.

Worth recording because it is the same shape as the bugs this project keeps hitting: a clean-looking measurement of something that never executed.

🤖 Generated with Claude Code

@PurHur

PurHur commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Maintainer review (2026-07-29) — do not merge yet

Verified on pr-24988 via ./script/docker-exec.sh:

ERROR: failed to apply php-vendor-implicit-nullable-84.patch
  Hint: git -C "/compiler" apply --check -p0 ".../php-vendor-implicit-nullable-84.patch"
error: patch failed: vendor/ircmaxell/php-cfg/lib/PHPCfg/Func.php:58
error: patch failed: vendor/ircmaxell/php-cfg/lib/PHPCfg/Parser.php:60

script/apply-patches.sh still exited 0 after printing ERROR (pre-existing skip noise + this failure), and bin/vm.php hello-world ran — so the green smoke does not prove the vendor patch applied.

Please regenerate the patch against current vendor/ircmaxell/php-cfg + php-optimizer (or add an idempotent “already nullable” skip), show git apply --check -p0 clean on a fresh composer install, and confirm a second apply-patches.sh is idempotent with zero ERROR: lines for this patch.

@PurHur

PurHur commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Gate fix (this push)

CI failed because php-vendor-implicit-nullable-84.patch was applied before php-cfg overlays, while its hunks were diffed against post-overlay vendor (Exit_ message field, propertyLazy, typed StaticVar, etc.). Fresh composer install → apply failed.

Fix

  • Regenerated the patch against post-overlay vendor (35 params / 23 files).
  • Moved apply_patch to after the php-cfg overlay fi.
  • Added patch_already_applied idempotency guard on ?Block $prior = null in AbstractVisitor.php.

Verified locally (php-compiler:22.04-dev)

rm -rf vendor/ircmaxell/php-cfg vendor/ircmaxell/php-optimizer
composer install --no-interaction --ignore-platform-reqs -q
script/apply-patches.sh   # Applied php-vendor-implicit-nullable-84.patch, rc=0
script/apply-patches.sh   # Skip … (already applied), rc=0
php bin/vm.php /tmp/hi.php  # hello, rc=0

Rebased onto current master. Ready to merge when recheck is green (or merge on local verification — remote CI is optional).

@PurHur
PurHur force-pushed the fix/php84-vendor-implicit-nullable-patch branch from bf4b186 to 847bc9a Compare July 29, 2026 20:42
anmichels and others added 2 commits July 29, 2026 20:42
Completes the implicit-nullable work started in #24972. PHP 8.4 deprecates
`Foo $x = null` and writes the notice to stderr, which the compliance harness
compares — so on an 8.4 host every case whose path touches one of these would
differ from Zend on stderr alone.

35 parameters across 23 files in php-cfg and php-optimizer, via the existing
patches/ layer (167 patches already there). `?Type` has been valid since PHP
7.1 and these declarations already accepted null, so behaviour is unchanged
on every supported version. The counterpart declarations in lib/Visitor/*
were fixed directly in #24972, so interface and implementations stay
consistent.

Measured in php-compiler:8.4-dev (PHP 8.4.23), bin/vm.php hello-world:

  implicit-nullable deprecations   16 -> 0
  total deprecations               17 -> 1

The one remaining is a different 8.4 deprecation class entirely — `Using "_"
as a class name is deprecated since 8.4` in ext/gettext/_.php — and is not
addressed here.

Verified:
  - apply-patches.sh rc=0, and rc=0 again on a second run (idempotent; the
    three "error"-matching lines are pre-existing "Skip ... (already
    applied)" messages whose PATCH NAMES contain "error", identical on master)
  - php-compiler:22.04-dev (PHP 8.2.32) with a fresh composer install:
    apply-patches rc=0, vm rc=0, stdout "hello 3", ZERO stderr
  - patch generated by diffing a pristine backup, then vendor restored and
    confirmed byte-identical before committing, so the patch is the only
    thing that applies the change

Note on process: the first attempt wired the apply_patch call into a
conditional helper whose anchor appears twice, the edit asserted and failed,
and the follow-up measurement showed 16 deprecations — which looked like the
patch not working rather than the patch never having run. Wired at top level
instead, next to the other unconditional php-llvm patches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hunks were diffed against post-overlay vendor (Exit_/Property/StaticVar
comments and signatures), so applying before php-cfg overlays failed on a
fresh composer install and broke the PR gate. Move the apply after the
php-cfg overlay block, regenerate the patch, and add an idempotency guard.

Co-authored-by: Cursor <cursoragent@cursor.com>
@PurHur
PurHur force-pushed the fix/php84-vendor-implicit-nullable-patch branch from 847bc9a to 73f63a8 Compare July 29, 2026 20:42
@PurHur
PurHur merged commit 5f977ca into master Jul 29, 2026
1 check failed
@PurHur
PurHur deleted the fix/php84-vendor-implicit-nullable-patch branch July 29, 2026 20:43
PurHur added a commit that referenced this pull request Jul 29, 2026
…ndor (#25008)

#24988 landed a patch whose Func/Property/Parser hunks did not match
post-overlay context (blank line / asymmetric-visibility fields), so
apply-patches.sh and north-star5-verify-fast failed on master. Regenerate
the 23-file diff against current overlaid vendor.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
PurHur added a commit that referenced this pull request Jul 30, 2026
)

The compiler gate has produced no master verdict at all. Measured over the 60
most recent runs of compiler-gate.yml:

  cancelled   31
  in flight   29
  completed    0

Every master push shared one concurrency group with cancel-in-progress, and
master takes ~12 commits/hour against a gate that needs ~25 min — so each run
was cancelled by the next merge before it could reach a conclusion. The gate
appeared to be running constantly and never reported anything.

That is the same failure as a check that reports without running, which this
repo has now hit four times (#24888 dead SKIPIF, #24726 quarantine from two
different commits, #24835 baseline that could not tell fixed from dropped,
#24933 skips counted as executions). Cancelled-forever is absence wearing a
busy spinner.

Master runs are now keyed on the SHA, so each merge gets its own verdict. PR
runs keep superseding each other on the ref, which is correct — once a branch
is pushed again the previous tree is nobody's concern.

Cost: master runs may overlap. With the paths filter (lib, ext, bin, patches,
the sweep scripts) and ~56% of commits touching lowering, that is roughly 7
runs/hour at ~25 min each, so ~3 concurrent. GitHub queues the excess.

Found while investigating why #24988 merged with a red gate. That failure was
`aot-smoke: 7 passed, 1 failed` — `echo` hitting rc=124, a 900s COMPILE
TIMEOUT under CI load, not a wrong result. Verified on current master
(e5bf674, which contains that patch): aot-smoke 8 passed, 0 failed locally,
including echo. So the red was infrastructure slowness, and master is intact —
but nobody could have known that from the gate, because it never finishes.

Co-authored-by: PurHur <tedyyyyy@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants