Skip to content

PFA: Fix strict_types checking when a const arg is inlined - #23256

Merged
arnaud-lb merged 1 commit into
php:masterfrom
arnaud-lb:pfa-bug-4
Aug 14, 2026
Merged

PFA: Fix strict_types checking when a const arg is inlined#23256
arnaud-lb merged 1 commit into
php:masterfrom
arnaud-lb:pfa-bug-4

Conversation

@arnaud-lb

Copy link
Copy Markdown
Member

GH-22829 added an optimization to burn literal arguments into the generated closure. An overlooked side effect is that this arguments are not checked anymore by zp_bind() (GH-22789).

Fix by checking these arguments ahead of time.

Bug found by Ryan @ Calif.io.

Comment threadZend/zend_partial.c
Comment on lines +581 to +583
zend_argument_type_error_ex(function, offset + 1,
"must be of type %s, %s given",
ZSTR_VAL(need_msg), zend_zval_value_name(&argv[offset]));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being thrown "at compile time"? As I imagine this could be unexpected.

@arnaud-lbarnaud-lbAug 13, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly at compile time. This code executes when a PFA expression is evaluated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then I can't see how this would differ in behaviour :)

@arnaud-lb
arnaud-lb marked this pull request as ready for review August 14, 2026 09:03

@GirgiasGirgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me.

@arnaud-lb
arnaud-lb merged commit 8b00475 into php:masterAug 14, 2026
18 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@arnaud-lb@Girgias