Implement parameter name inheritance

nikic · nikic · commit 3e2eac93cce3 · 2020-07-02T18:52:35.000+02:00
diff --git a/Zend/tests/named_params/lsp_conflict_1.phpt b/Zend/tests/named_params/lsp_conflict_1.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Parameter name conflict: Direct inheritance conflict
+--FILE--
+<?php
+
+class A {
+    public function method($a, $b) {}
+}
+class B extends A {
+    public function method($b, $a) {}
+}
+
+?>
+--EXPECTF--
+Fatal error: Parameter $a of B::method() at position #2 conflicts with parameter $a of A::method() at position #1 in %s on line %d
diff --git a/Zend/tests/named_params/lsp_conflict_2.phpt b/Zend/tests/named_params/lsp_conflict_2.phpt
@@ -0,0 +1,18 @@
+--TEST--
+Parameter name conflict: Indirect conflict between two interfaces
+--FILE--
+<?php
+
+interface I {
+    public function method($a, $b);
+}
+interface J {
+    public function method($b, $a);
+}
+class C implements I, J {
+    public function method($x, $y) {}
+}
+
+?>
+--EXPECTF--
+Fatal error: Parameter $b of C::method() at position #2 (inherited from a different prototype method) conflicts with parameter $b of J::method() at position #1 in %s on line %d
diff --git a/Zend/tests/named_params/lsp_no_conflict.phpt b/Zend/tests/named_params/lsp_no_conflict.phpt
@@ -0,0 +1,60 @@
+--TEST--
+Can use parameter names from proto methods
+--FILE--
+<?php
+
+class A {
+    public function method($a) {
+        echo __METHOD__ . ": $a\n";
+    }
+}
+class B extends A {
+    public function method($b) {
+        echo __METHOD__ . ": $b\n";
+    }
+}
+class C extends B {
+    public function method($c) {
+        echo __METHOD__ . ": $c\n";
+    }
+}
+
+(new B)->method(a: 42);
+(new B)->method(b: 42);
+(new C)->method(a: 42);
+(new C)->method(b: 42);
+(new C)->method(c: 42);
+echo "\n";
+
+interface I {
+    public function method($i);
+}
+interface J {
+    public function method($j);
+}
+interface K {
+    public function method($k);
+}
+class D implements I, J, K {
+    public function method($d) {
+        echo __METHOD__ . ": $d\n";
+    }
+}
+
+(new D)->method(i: 42);
+(new D)->method(j: 42);
+(new D)->method(k: 42);
+(new D)->method(d: 42);
+
+?>
+--EXPECT--
+B::method: 42
+B::method: 42
+C::method: 42
+C::method: 42
+C::method: 42
+
+D::method: 42
+D::method: 42
+D::method: 42
+D::method: 42
diff --git a/Zend/zend_API.c b/Zend/zend_API.c
@@ -2092,6 +2092,8 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio
 		internal_function->function_name = zend_string_init_interned(ptr->fname, fname_len, 1);
 		internal_function->scope = scope;
 		internal_function->prototype = NULL;
+		internal_function->attributes = NULL;
+		internal_function->arg_names = NULL;
 		if (ptr->flags) {
 			if (!(ptr->flags & ZEND_ACC_PPP_MASK)) {
 				if (ptr->flags != ZEND_ACC_DEPRECATED && scope) {
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -418,6 +418,7 @@ struct _zend_op_array {
 	uint32_t required_num_args;
 	zend_arg_info *arg_info;
 	HashTable *attributes;
+	HashTable *arg_names;
 	/* END of common elements */
 
 	int cache_size;     /* number of run_time_cache_slots * sizeof(void*) */
@@ -468,6 +469,7 @@ typedef struct _zend_internal_function {
 	uint32_t required_num_args;
 	zend_internal_arg_info *arg_info;
 	HashTable *attributes;
+	HashTable *arg_names;
 	/* END of common elements */
 
 	zif_handler handler;
@@ -491,7 +493,14 @@ union _zend_function {
 		uint32_t num_args;
 		uint32_t required_num_args;
 		zend_arg_info *arg_info;  /* index -1 represents the return value info, if any */
-		HashTable   *attributes;
+		HashTable *attributes;
+		/* Mapping from argument names to argument offset for named arguments.
+		 * To reduce memory usage, this is only used if either:
+		 *  * We need to preserve argument names from a parent method. (TODO)
+		 *  * There are many arguments. (TODO)
+		 * Otherwise, we will simplify perform a linear scan over the arg_info.
+		 */
+		HashTable *arg_names;
 	} common;
 
 	zend_op_array op_array;
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -138,6 +138,7 @@ ZEND_API const zend_internal_function zend_pass_function = {
 	0,                      /* required_num_args */
 	NULL,                   /* arg_info          */
 	NULL,                   /* attributes        */
+	NULL,                   /* arg_names         */
 	ZEND_FN(pass),          /* handler           */
 	NULL,                   /* module            */
 	{NULL,NULL,NULL,NULL}   /* reserved          */
@@ -4294,7 +4295,16 @@ static zend_always_inline uint32_t zend_get_arg_offset_by_name(
 		return *(uintptr_t *)(cache_slot + 1);
 	}
 
-	// TODO: Use a hash table?
+	if (fbc->common.arg_names) {
+		zval *zv = zend_hash_find(fbc->common.arg_names, arg_name);
+		if (zv) {
+			*cache_slot = fbc;
+			*(uintptr_t *)(cache_slot + 1) = Z_LVAL_P(zv);
+			return Z_LVAL_P(zv);
+		}
+		return (uint32_t) - 1;
+	}
+
 	uint32_t num_args = fbc->common.num_args;
 	if (fbc->type == ZEND_USER_FUNCTION) {
 		for (uint32_t i = 0; i < num_args; i++) {
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -524,6 +524,93 @@ static inheritance_status zend_do_perform_arg_type_hint_check(
 }
 /* }}} */
 
+static const char *get_arg_name(const zend_function *func, const zend_arg_info *info, size_t *len) {
+	if (func->type == ZEND_INTERNAL_FUNCTION) {
+		zend_internal_arg_info *internal_info = (zend_internal_arg_info *) info;
+		*len = strlen(internal_info->name);
+		return internal_info->name;
+	} else {
+		*len = ZSTR_LEN(info->name);
+		return ZSTR_VAL(info->name);
+	}
+}
+
+static void add_arg_name(
+		HashTable *ht, const zend_function *to, const zend_function *from,
+		zend_string *arg_name, uint32_t arg_offset) {
+	zval *existing_arg_offset_zv = zend_hash_find(ht, arg_name);
+	if (existing_arg_offset_zv) {
+		uint32_t existing_arg_offset = Z_LVAL_P(existing_arg_offset_zv);
+		if (existing_arg_offset != arg_offset) {
+			/* Determine whether the conflict is with "to" itself, or because we inherited from
+			 * a different prototype method. */
+			/* TODO: Determine the exact conflicting interface/method */
+			size_t to_arg_name_len;
+			const char *to_arg_name =
+				get_arg_name(to, &to->common.arg_info[existing_arg_offset], &to_arg_name_len);
+			zend_bool is_direct_conflict =
+				ZSTR_LEN(arg_name) == to_arg_name_len
+				&& !memcmp(ZSTR_VAL(arg_name), to_arg_name, to_arg_name_len);
+			zend_error_noreturn(E_COMPILE_ERROR,
+				"Parameter $%s of %s::%s() at position #%" PRIu32 "%s conflicts with "
+				"parameter $%s of %s::%s() at position #%" PRIu32,
+				ZSTR_VAL(arg_name),
+				ZSTR_VAL(to->common.scope->name), ZSTR_VAL(to->common.function_name),
+				existing_arg_offset + 1,
+				is_direct_conflict ? "" : " (inherited from a different prototype method)",
+				ZSTR_VAL(arg_name),
+				ZSTR_VAL(from->common.scope->name), ZSTR_VAL(from->common.function_name),
+				arg_offset + 1);
+		}
+	} else {
+		zval arg_offset_zv;
+		ZVAL_LONG(&arg_offset_zv, arg_offset);
+		zend_hash_add_new(ht, arg_name, &arg_offset_zv);
+	}
+}
+
+static void add_arg_names(zend_function *to, const zend_function *from) {
+	zend_bool persistent = to->type == ZEND_INTERNAL_FUNCTION;
+	HashTable *ht = to->common.arg_names;
+	zend_string *name;
+	zval *val;
+
+	if (!ht) {
+		ht = pemalloc(sizeof(HashTable), persistent);
+		zend_hash_init(ht, to->common.num_args, NULL, NULL, persistent);
+	}
+
+	if (from->common.arg_names) {
+		ZEND_HASH_FOREACH_STR_KEY_VAL(from->common.arg_names, name, val) {
+			uint32_t arg_offset = Z_LVAL_P(val);
+			if (arg_offset >= to->common.num_args) {
+				/* Don't inherit argument names that have been subsumed by a variadic. */
+				continue;
+			}
+
+			add_arg_name(ht, to, from, name, arg_offset);
+		} ZEND_HASH_FOREACH_END();
+	} else {
+		for (uint32_t i = 0; i < from->common.num_args; i++) {
+			if (i >= to->common.num_args) {
+				/* Don't inherit argument names that have been subsumed by a variadic. */
+				continue;
+			}
+
+			const zend_arg_info *info = &from->common.arg_info[i];
+			const zend_internal_arg_info *internal_info = (const zend_internal_arg_info *) info;
+			zend_string *arg_name = from->type == ZEND_INTERNAL_FUNCTION
+				? zend_string_init(internal_info->name, strlen(internal_info->name), persistent)
+				: zend_string_copy(info->name);
+			add_arg_name(ht, to, from, arg_name, i);
+			zend_string_release(arg_name);
+		}
+	}
+	if (!to->common.arg_names) {
+		to->common.arg_names = ht;
+	}
+}
+
 /* For abstract trait methods, proto_scope will differ from proto->common.scope,
  * as self will refer to the self of the class the trait is used in, not the trait
  * the method was declared in. */
@@ -533,7 +620,7 @@ static inheritance_status zend_do_perform_implementation_check(
 {
 	uint32_t i, num_args, proto_num_args, fe_num_args;
 	inheritance_status status, local_status;
-	zend_bool proto_is_variadic, fe_is_variadic;
+	zend_bool proto_is_variadic, fe_is_variadic, needs_arg_name_table;
 
 	/* Checks for constructors only if they are declared in an interface,
 	 * or explicitly marked as abstract
@@ -570,6 +657,7 @@ static inheritance_status zend_do_perform_implementation_check(
 	proto_num_args = proto->common.num_args + proto_is_variadic;
 	fe_num_args = fe->common.num_args + fe_is_variadic;
 	num_args = MAX(proto_num_args, fe_num_args);
+	needs_arg_name_table = proto->common.arg_names != NULL;
 
 	status = INHERITANCE_SUCCESS;
 	for (i = 0; i < num_args; i++) {
@@ -605,6 +693,27 @@ static inheritance_status zend_do_perform_implementation_check(
 		if (ZEND_ARG_SEND_MODE(fe_arg_info) != ZEND_ARG_SEND_MODE(proto_arg_info)) {
 			return INHERITANCE_ERROR;
 		}
+
+		/* Only check this for non-variadic parameters. */
+		if (i < fe->common.num_args && !needs_arg_name_table) {
+			size_t proto_arg_name_len, fe_arg_name_len;
+			const char *proto_arg_name = get_arg_name(proto, proto_arg_info, &proto_arg_name_len);
+			const char *fe_arg_name = get_arg_name(fe, fe_arg_info, &fe_arg_name_len);
+			if (proto_arg_name_len != fe_arg_name_len
+					|| memcmp(proto_arg_name, fe_arg_name, fe_arg_name_len) != 0) {
+				needs_arg_name_table = 1;
+			}
+		}
+	}
+
+	if (needs_arg_name_table) {
+		/* Parameter names are not the same! */
+		/* TODO: HACK!!! */
+		zend_function *fe_ = (zend_function *) fe;
+		if (!fe->common.arg_names) {
+			add_arg_names(fe_, fe);
+		}
+		add_arg_names(fe_, proto);
 	}
 
 	/* Check return type compatibility, but only if the prototype already specifies
diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c
@@ -64,6 +64,7 @@ void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_siz
 	op_array->filename = zend_get_compiled_filename();
 	op_array->doc_comment = NULL;
 	op_array->attributes = NULL;
+	op_array->arg_names = NULL;
 
 	op_array->arg_info = NULL;
 	op_array->num_args = 0;
@@ -502,6 +503,10 @@ ZEND_API void destroy_op_array(zend_op_array *op_array)
 	if (op_array->attributes) {
 		zend_hash_release(op_array->attributes);
 	}
+	if (op_array->arg_names) {
+		zend_hash_destroy(op_array->arg_names);
+		efree(op_array->arg_names);
+	}
 	if (op_array->live_range) {
 		efree(op_array->live_range);
 	}
