Address some code review comments

Author: kocsismate

Zend/tests/clone_initializer/clone_initializer_success9.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Test that clone initializer list is displayed properly in assert()
+--INI--
+zend.assertions = 1
+assert.warning = 0
+assert.exception = 1
+--FILE--
+<?php
+
+try {
+    assert(clone new stdClass() === null);
+} catch (AssertionError $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    assert(clone new stdClass() with {} === null);
+} catch (AssertionError $e) {
+    echo $e->getMessage() . "\n";
+}
+
+try {
+    assert(clone new stdClass() with {foo: 1, bar: "abc"} === null);
+} catch (AssertionError $e) {
+    echo $e->getMessage() . "\n";
+}
+
+?>
+--EXPECT--
+assert(clone new stdClass() === null)
+assert(clone new stdClass() with {} === null)
+assert(clone new stdClass() with {foo: 1, bar: 'abc'} === null)

Zend/zend_API.c

@@ -4191,6 +4191,10 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
 
 	if (ZEND_TYPE_IS_SET(type)) {
 		ce->ce_flags |= ZEND_ACC_HAS_TYPE_HINTS;
+
+		if (access_type & ZEND_ACC_READONLY) {
+			ce->ce_flags |= ZEND_ACC_HAS_READONLY_PROPS;
+		}
 	}
 
 	if (ce->type == ZEND_INTERNAL_CLASS) {

Zend/zend_ast.c

@@ -1833,10 +1833,16 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
 			zend_ast_export_list(str, (zend_ast_list*)ast, 0, 0, indent);
 			break;
 		case ZEND_AST_PROPERTY_INITIALIZER_LIST:
-			smart_str_appends(str, " {");
-			zend_ast_export_list(str, (zend_ast_list*)ast, 0, 0, indent);
-			smart_str_appends(str, " }");
+			smart_str_appends(str, "{");
+			zend_ast_export_list(str, (zend_ast_list*)ast, 1, 0, indent);
+			smart_str_appends(str, "}");
+			break;
+		case ZEND_AST_INITIALIZER_EXPR: {
+			zend_ast_export_name(str, ast->child[0], 0, indent);
+			smart_str_appends(str, ": ");
+			zend_ast_export_ex(str, ast->child[1], priority, indent);
 			break;
+		}
 		case ZEND_AST_CLOSURE_USES:
 			smart_str_appends(str, " use(");
 			zend_ast_export_var_list(str, (zend_ast_list*)ast, indent);
@@ -1963,8 +1969,6 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
 			}
 			smart_str_appendc(str, '`');
 			break;
-		case ZEND_AST_CLONE:
-			PREFIX_OP("clone ", 270, 271);
 		case ZEND_AST_EXIT:
 			if (ast->child[0]) {
 				FUNC_OP("exit");
@@ -2160,6 +2164,16 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
 				smart_str_appendc(str, ')');
 			}
 			break;
+		case ZEND_AST_CLONE:
+			if (ast->child[1]) {
+				smart_str_appends(str, "clone ");
+				zend_ast_export_ex(str, ast->child[0], priority, indent);
+				smart_str_appends(str, " with ");
+				zend_ast_export_ex(str, ast->child[1], priority, indent);
+			} else {
+				PREFIX_OP("clone ", 270, 271);
+			}
+			break;
 		case ZEND_AST_INSTANCEOF:
 			zend_ast_export_ex(str, ast->child[0], 0, indent);
 			smart_str_appends(str, " instanceof ");

Zend/zend_compile.c

@@ -4836,7 +4836,7 @@ static void zend_compile_clone(znode *result, zend_ast *ast) /* {{{ */
 			znode property_name_node;
 			property_name_node.op_type = IS_CONST;
 			ZVAL_STR_COPY(&property_name_node.u.constant, property_name);
-			opline = zend_emit_op(result, ZEND_INIT_OBJ, &property_object_node, &property_name_node);
+			opline = zend_emit_op(result, ZEND_CLONE_INIT_PROP, &property_object_node, &property_name_node);
 			opline->result.var  = get_temporary_variable();
 			opline->extended_value = zend_alloc_cache_slots(3);
 

Zend/zend_compile.h

@@ -240,7 +240,7 @@ typedef struct _zend_oparray_context {
 /* or IS_CONSTANT_VISITED_MARK                            |     |     |     */
 #define ZEND_CLASS_CONST_IS_CASE         (1 << 6)  /*     |     |     |  X  */
 /*                                                        |     |     |     */
-/* Class Flags (unused: 21,30,31)                         |     |     |     */
+/* Class Flags (unused: 30,31)                            |     |     |     */
 /* ===========                                            |     |     |     */
 /*                                                        |     |     |     */
 /* Special class types                                    |     |     |     */
@@ -288,6 +288,9 @@ typedef struct _zend_oparray_context {
 /* Class is linked apart from variance obligations.       |     |     |     */
 #define ZEND_ACC_NEARLY_LINKED           (1 << 20) /*  X  |     |     |     */
 /*                                                        |     |     |     */
+/* Class has readonly props                               |     |     |     */
+#define ZEND_ACC_HAS_READONLY_PROPS      (1 << 21) /*  X  |     |     |     */
+/*                                                        |     |     |     */
 /* stored in opcache (may be partially)                   |     |     |     */
 #define ZEND_ACC_CACHED                  (1 << 22) /*  X  |     |     |     */
 /*                                                        |     |     |     */

Zend/zend_enum.c

@@ -46,6 +46,7 @@ zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case
 		zval *prop = OBJ_PROP_NUM(zobj, 1);
 
 		ZVAL_COPY(prop, backing_value_zv);
+		/* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */
 		Z_PROP_FLAG_P(prop) = 0;
 	}
 

Zend/zend_execute.h

@@ -454,6 +454,7 @@ ZEND_API int ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *call);
 	} while (0)
 
 #define ZEND_CLASS_HAS_TYPE_HINTS(ce) ((ce->ce_flags & ZEND_ACC_HAS_TYPE_HINTS) == ZEND_ACC_HAS_TYPE_HINTS)
+#define ZEND_CLASS_HAS_READONLY_PROPS(ce) ((ce->ce_flags & ZEND_ACC_HAS_READONLY_PROPS) == ZEND_ACC_HAS_READONLY_PROPS)
 
 ZEND_API bool zend_verify_property_type(zend_property_info *info, zval *property, bool strict);
 ZEND_COLD void zend_verify_property_type_error(zend_property_info *info, zval *property);

Zend/zend_inheritance.c

@@ -1628,7 +1628,7 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par
 			ce->ce_flags |= ZEND_ACC_EXPLICIT_ABSTRACT_CLASS;
 		}
 	}
-	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES);
+	ce->ce_flags |= parent_ce->ce_flags & (ZEND_HAS_STATIC_IN_METHODS | ZEND_ACC_HAS_TYPE_HINTS | ZEND_ACC_HAS_READONLY_PROPS | ZEND_ACC_USE_GUARDS | ZEND_ACC_NOT_SERIALIZABLE | ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES);
 }
 /* }}} */
 

Zend/zend_object_handlers.c

@@ -816,7 +816,7 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 				if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
 					if (UNEXPECTED((Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE)) ||
 						(UNEXPECTED(EG(current_execute_data) && EG(current_execute_data)->opline &&
-						EG(current_execute_data)->opline->opcode == ZEND_INIT_OBJ))
+						EG(current_execute_data)->opline->opcode == ZEND_CLONE_INIT_PROP))
 					) {
 						Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE;
 					} else {

Zend/zend_objects.c

@@ -266,9 +266,8 @@ ZEND_API void ZEND_FASTCALL zend_objects_clone_members(zend_object *new_object,
 		GC_ADDREF(new_object);
 		zend_call_known_instance_method_with_0_params(new_object->ce->clone, new_object, NULL);
 
-		for (uint32_t i = 0; i < new_object->ce->default_properties_count; i++) {
-			zend_property_info *prop_info = new_object->ce->properties_info_table[i];
-			if (prop_info && (prop_info->flags & ZEND_ACC_READONLY)) {
+		if (ZEND_CLASS_HAS_READONLY_PROPS(new_object->ce)) {
+			for (uint32_t i = 0; i < new_object->ce->default_properties_count; i++) {
 				zval *prop = OBJ_PROP_NUM(new_object, i);
 				Z_PROP_FLAG_P(prop) &= ~IS_PROP_REINITABLE;
 			}

Zend/zend_types.h

@@ -1438,7 +1438,7 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
  * the Z_EXTRA space when copying property default values etc. We define separate
  * macros for this purpose, so this workaround is easier to remove in the future. */
 #define IS_PROP_UNINIT 1
-#define IS_PROP_REINITABLE 2
+#define IS_PROP_REINITABLE 2 /* It has impact only on readonly properties */
 #define Z_PROP_FLAG_P(z) Z_EXTRA_P(z)
 #define ZVAL_COPY_VALUE_PROP(z, v) \
 	do { *(z) = *(v); } while (0)

Zend/zend_vm_def.h

@@ -9397,7 +9397,7 @@ ZEND_VM_HANDLER(202, ZEND_CALLABLE_CONVERT, UNUSED, UNUSED)
 	ZEND_VM_NEXT_OPCODE();
 }
 
-ZEND_VM_HANDLER(203, ZEND_INIT_OBJ, VAR|UNUSED|THIS|CV, CONST, CACHE_SLOT, SPEC(OP_DATA=CONST|TMP|VAR|CV))
+ZEND_VM_HANDLER(203, ZEND_CLONE_INIT_PROP, VAR, CONST, CACHE_SLOT, SPEC(OP_DATA=CONST|TMP|VAR|CV))
 {
 	USE_OPLINE
 	zval *object, *value, tmp;
@@ -9411,14 +9411,14 @@ ZEND_VM_HANDLER(203, ZEND_INIT_OBJ, VAR|UNUSED|THIS|CV, CONST, CACHE_SLOT, SPEC(
 	if (OP1_TYPE != IS_UNUSED && UNEXPECTED(Z_TYPE_P(object) != IS_OBJECT)) {
 		if (Z_ISREF_P(object) && Z_TYPE_P(Z_REFVAL_P(object)) == IS_OBJECT) {
 			object = Z_REFVAL_P(object);
-			ZEND_VM_C_GOTO(init_object);
+			ZEND_VM_C_GOTO(init_prop);
 		}
 		zend_throw_non_object_error(object, GET_OP2_ZVAL_PTR(BP_VAR_R) OPLINE_CC EXECUTE_DATA_CC);
 		value = &EG(uninitialized_zval);
-		ZEND_VM_C_GOTO(free_and_exit_init_obj);
+		ZEND_VM_C_GOTO(free_and_exit_init_prop);
 	}
 
-ZEND_VM_C_LABEL(init_object):
+ZEND_VM_C_LABEL(init_prop):
 	zobj = Z_OBJ_P(object);
 	void **cache_slot = CACHE_ADDR(opline->extended_value);
 	if (EXPECTED(zobj->ce == CACHED_PTR(opline->extended_value))) {
@@ -9439,14 +9439,14 @@ ZEND_VM_C_LABEL(init_object):
 					if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) {
 						Z_PROP_FLAG_P(property_val) &= ~IS_PROP_REINITABLE;
 					}
-					ZEND_VM_C_GOTO(free_and_exit_init_obj);
+					ZEND_VM_C_GOTO(free_and_exit_init_prop);
 				} else {
-ZEND_VM_C_LABEL(fast_init_obj):
-					value = zend_assign_to_variable(property_val, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
+ZEND_VM_C_LABEL(fast_init_prop):
+					zend_assign_to_variable(property_val, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
 					if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
-						ZVAL_COPY(EX_VAR(opline->result.var), value);
+						ZVAL_COPY(EX_VAR(opline->result.var), object);
 					}
-					ZEND_VM_C_GOTO(exit_init_obj);
+					ZEND_VM_C_GOTO(exit_init_prop);
 				}
 			}
 		} else {
@@ -9460,7 +9460,7 @@ ZEND_VM_C_LABEL(fast_init_obj):
 				}
 				property_val = zend_hash_find_known_hash(zobj->properties, name);
 				if (property_val) {
-					ZEND_VM_C_GOTO(fast_init_obj);
+					ZEND_VM_C_GOTO(fast_init_prop);
 				}
 			}
 
@@ -9494,9 +9494,9 @@ ZEND_VM_C_LABEL(fast_init_obj):
 				}
 				zend_hash_add_new(zobj->properties, name, value);
 				if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
-					ZVAL_COPY(EX_VAR(opline->result.var), value);
+					ZVAL_COPY(EX_VAR(opline->result.var), object);
 				}
-				ZEND_VM_C_GOTO(exit_init_obj);
+				ZEND_VM_C_GOTO(free_and_exit_init_prop);
 			}
 		}
 	}
@@ -9509,15 +9509,15 @@ ZEND_VM_C_LABEL(fast_init_obj):
 
 	value = zobj->handlers->write_property(zobj, name, value, cache_slot);
 
-ZEND_VM_C_LABEL(free_and_exit_init_obj):
+ZEND_VM_C_LABEL(free_and_exit_init_prop):
 	if (EXPECTED(RETURN_VALUE_USED(opline))) {
 		ZVAL_COPY_DEREF(EX_VAR(opline->result.var), object);
 	}
 	FREE_OP_DATA();
-ZEND_VM_C_LABEL(exit_init_obj):
+ZEND_VM_C_LABEL(exit_init_prop):
 	FREE_OP2();
 	FREE_OP1();
-	/* init_obj has two opcodes! */
+	/* ZEND_CLONE_INIT_PROP has two opcodes! */
 	ZEND_VM_NEXT_OPCODE_EX(1, 2);
 }