Skip to content

Conversation

@alexandre-daubois
Copy link
Member

Missing deprecation raised in #20857

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

See minor note.
Everything else looks fine.

static inline zend_result ct_eval_isset_dim(zval *result, uint32_t extended_value, zval *op1, zval *op2) {
if (Z_TYPE_P(op2) == IS_NULL) {
return FAILURE;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this still necessary? Shouldn't fetch_array_elem() already handle it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would say it is. If op1 is not an array (e.g. isset($a[null])), the condition below this one (if (Z_TYPE_P(op1) == IS_ARRAY || IS_PARTIAL_ARRAY(op1)) {) would be false and we would completely ditch any check on a null key.

Adding this early return avoids compile-time checks that would silence this kind of operation. If I get it right 😄

Copy link
Member

Choose a reason for hiding this comment

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

The deprecation was only added for arrays though, as far as I understand.

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.

3 participants