Skip to content

Deprecate using null as an array offset and when calling array_key_exists() #19511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Aug 18, 2025

@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 3 times, most recently from 472c3c6 to baeda1f Compare August 20, 2025 08:30
@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 2 times, most recently from d5fd9e5 to a602268 Compare August 20, 2025 15:02
@alexandre-daubois
Copy link
Member Author

Still a few tests to fix but we're getting closer

@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 2 times, most recently from 6f50c89 to e508fcc Compare August 21, 2025 06:54
@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 4 times, most recently from 8f22921 to dc5049b Compare August 21, 2025 08:54
@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 2 times, most recently from c47e2b5 to 58878be Compare August 21, 2025 11:50
@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 5 times, most recently from 2175fc1 to 636861f Compare August 21, 2025 13:06
@alexandre-daubois
Copy link
Member Author

alexandre-daubois commented Aug 21, 2025

JIT really is something. But PR is now passing tests

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

I hate the extra state protections we have to do but I think we can simplify some of them.
This is why I don't like these kinds of deprecations, lots of pointless crap needs to be added to protect the engine. (Granted, this is not your fault, this is caused by a design problem of error handlers)

@@ -500,6 +500,31 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim,
}
ZEND_FALLTHROUGH;
case IS_NULL:
/* The array may be destroyed while throwing the notice.
Copy link
Member

Choose a reason for hiding this comment

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

Please, no.
I would prefer if you just fetched the result first, copied it to the return value zval, and then emitted the deprecation. Similarly in many other places (this won't be possible for all code because some return a zval* to the array slot).

Sign up for free to 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.

5 participants