Skip to content

Commit ff11459

Browse files
committed
Refactor spl_array_has_dimension_ex()
Use early returns instead of else blocks Add comments, especially to explain why we need a check_empty == 2 check
1 parent 22d700b commit ff11459

File tree

1 file changed

+24
-22
lines changed

1 file changed

+24
-22
lines changed

ext/spl/spl_array.c

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -578,25 +578,26 @@ static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{
578578
spl_array_unset_dimension_ex(1, object, offset);
579579
} /* }}} */
580580

581-
static int spl_array_has_dimension_ex(int check_inherited, zend_object *object, zval *offset, int check_empty) /* {{{ */
581+
static int spl_array_has_dimension_ex(bool check_inherited, zend_object *object, zval *offset, int check_empty) /* {{{ */
582582
{
583583
spl_array_object *intern = spl_array_from_obj(object);
584584
zval rv, *value = NULL, *tmp;
585585

586586
if (check_inherited && intern->fptr_offset_has) {
587587
zend_call_method_with_1_params(object, object->ce, &intern->fptr_offset_has, "offsetExists", &rv, offset);
588588

589-
if (zend_is_true(&rv)) {
590-
zval_ptr_dtor(&rv);
591-
if (check_empty != 1) {
592-
return 1;
593-
} else if (intern->fptr_offset_get) {
594-
value = spl_array_read_dimension_ex(1, object, offset, BP_VAR_R, &rv);
595-
}
596-
} else {
589+
if (!zend_is_true(&rv)) {
597590
zval_ptr_dtor(&rv);
598591
return 0;
599592
}
593+
zval_ptr_dtor(&rv);
594+
595+
/* For isset calls we don't need to check the value, so return early */
596+
if (!check_empty) {
597+
return 1;
598+
} else if (intern->fptr_offset_get) {
599+
value = spl_array_read_dimension_ex(1, object, offset, BP_VAR_R, &rv);
600+
}
600601
}
601602

602603
if (!value) {
@@ -615,33 +616,34 @@ static int spl_array_has_dimension_ex(int check_inherited, zend_object *object,
615616
tmp = zend_hash_index_find(ht, key.h);
616617
}
617618

618-
if (tmp) {
619-
if (check_empty == 2) {
620-
return 1;
621-
}
622-
} else {
619+
if (!tmp) {
623620
return 0;
624621
}
625622

623+
/* check_empty is only equal to 2 if it is called from offsetExists on this class,
624+
* where it needs to report an offset exists even if the value is null */
625+
if (check_empty == 2) {
626+
return 1;
627+
}
628+
626629
if (check_empty && check_inherited && intern->fptr_offset_get) {
627630
value = spl_array_read_dimension_ex(1, object, offset, BP_VAR_R, &rv);
628631
} else {
629632
value = tmp;
630633
}
631634
}
632635

633-
{
634-
bool result = check_empty ? zend_is_true(value) : Z_TYPE_P(value) != IS_NULL;
635-
if (value == &rv) {
636-
zval_ptr_dtor(&rv);
637-
}
638-
return result;
636+
if (value == &rv) {
637+
zval_ptr_dtor(&rv);
639638
}
639+
640+
/* empty() check the value is not falsy, isset() only check it is not null */
641+
return check_empty ? zend_is_true(value) : Z_TYPE_P(value) != IS_NULL;
640642
} /* }}} */
641643

642644
static int spl_array_has_dimension(zend_object *object, zval *offset, int check_empty) /* {{{ */
643645
{
644-
return spl_array_has_dimension_ex(1, object, offset, check_empty);
646+
return spl_array_has_dimension_ex(/* check_inherited */ true, object, offset, check_empty);
645647
} /* }}} */
646648

647649
/* {{{ Returns whether the requested $index exists. */
@@ -651,7 +653,7 @@ PHP_METHOD(ArrayObject, offsetExists)
651653
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &index) == FAILURE) {
652654
RETURN_THROWS();
653655
}
654-
RETURN_BOOL(spl_array_has_dimension_ex(0, Z_OBJ_P(ZEND_THIS), index, 2));
656+
RETURN_BOOL(spl_array_has_dimension_ex(/* check_inherited */ false, Z_OBJ_P(ZEND_THIS), index, 2));
655657
} /* }}} */
656658

657659
/* {{{ Returns the value at the specified $index. */

0 commit comments

Comments
 (0)