Variadic placeholder should be last (including after named args)

arnaud-lb · arnaud-lb · commit 6c5bdf7c29c8 · 2025-11-03T19:54:50.000+01:00
diff --git a/Zend/Optimizer/zend_call_graph.c b/Zend/Optimizer/zend_call_graph.c
@@ -151,6 +151,9 @@ ZEND_API void zend_analyze_calls(zend_arena **arena, zend_script *script, uint32
 						call_info->named_args = true;
 						break;
 					}
+					if (opline->opcode == ZEND_SEND_PLACEHOLDER && opline->op1.num == _IS_PLACEHOLDER_VARIADIC) {
+						break;
+					}
 
 					uint32_t num = opline->op2.num;
 					if (num > 0) {
diff --git a/Zend/tests/partial_application/compile_errors_002.phpt b/Zend/tests/partial_application/compile_errors_002.phpt
@@ -1,8 +1,8 @@
 --TEST--
-Closure application compile errors: only named arguments after ...
+Closure application compile errors: ... must be last
 --FILE--
 <?php
 foo(..., ?);
 ?>
 --EXPECTF--
-Fatal error: Only named arguments may follow variadic placeholder in %s on line %d
+Fatal error: Variadic placeholder must be last in %s on line %d
diff --git a/Zend/tests/partial_application/compile_errors_004.phpt b/Zend/tests/partial_application/compile_errors_004.phpt
@@ -1,8 +1,8 @@
 --TEST--
-Closure application compile errors: named arguments must come after variadic placeholder
+Closure application compile errors: named arguments must come before variadic placeholder
 --FILE--
 <?php
-foo(n: 5, ...);
+foo(..., n: 5);
 ?>
 --EXPECTF--
-Fatal error: Cannot use positional argument after named argument in %s on line %d
+Fatal error: Variadic placeholder must be last in %s on line %d
diff --git a/Zend/tests/partial_application/compile_errors_005.phpt b/Zend/tests/partial_application/compile_errors_005.phpt
@@ -5,4 +5,4 @@ Closure application compile errors: follow variadic with un-named arg
 foo(..., $a);
 ?>
 --EXPECTF--
-Fatal error: Only named arguments may follow variadic placeholder in %s on line %d
+Fatal error: Variadic placeholder must be last in %s on line %d
diff --git a/Zend/tests/partial_application/compile_errors_007.phpt b/Zend/tests/partial_application/compile_errors_007.phpt
@@ -1,8 +1,8 @@
 --TEST--
-Closure application compile errors: mix application with unpack (placeholder before)
+Closure application compile errors: mix application with unpack
 --FILE--
 <?php
-foo(..., ...["foo" => "bar"]);
+foo(...["foo" => "bar"], ...);
 ?>
 --EXPECTF--
 Fatal error: Cannot combine partial application and unpacking %s on line %d
diff --git a/Zend/tests/partial_application/extra_collect_001.phpt b/Zend/tests/partial_application/extra_collect_001.phpt
@@ -6,23 +6,23 @@ function foo(...$args) {
     var_dump($args);
 }
 
-$foo = foo(..., foo: "foo");
+$foo = foo(foo: "foo", ...);
 
-$bar = $foo(..., bar: "bar");
+$bar = $foo(bar: "bar", ...);
 
-$baz = $bar(..., baz: "baz");
+$baz = $bar(baz: "baz", ...);
 
 $baz();
 
 $foo = foo(...);
 
-$bar = $foo(..., bar: "bar");
+$bar = $foo(bar: "bar", ...);
 
-$baz = $bar(..., baz: "baz");
+$baz = $bar(baz: "baz", ...);
 
 $baz();
 
-$foo = foo(..., foo: "foo");
+$foo = foo(foo: "foo", ...);
 
 $foo(bar: "bar");
 ?>
diff --git a/Zend/tests/partial_application/fuzz_004.phpt b/Zend/tests/partial_application/fuzz_004.phpt
@@ -6,7 +6,7 @@ function foo($a, $b) {
     return $a + $b;
 }
 
-$foo = foo(..., b: 10);
+$foo = foo(b: 10, ...);
 
 try {
     $foo->__invoke(UNDEFINED);
diff --git a/Zend/tests/partial_application/fuzz_005.phpt b/Zend/tests/partial_application/fuzz_005.phpt
@@ -6,9 +6,9 @@ Closure application fuzz 005
 function foo(int $day = 1, int $month = 1, int $yearh = 1, int $year = 2005) {
 }
 
-$foo = foo(..., month: 12);
+$foo = foo(month: 12, ...);
 
-$bar = $foo(..., year: 2016);
+$bar = $foo(year: 2016, ...);
 
 ?>
 ==DONE==
diff --git a/Zend/tests/partial_application/fuzz_006.phpt b/Zend/tests/partial_application/fuzz_006.phpt
@@ -7,7 +7,7 @@ function foo(int $day = 1, int $month = 1, int $year = 2005) {
     var_dump($day);
 }
 
-$foo = foo(..., month: 12);
+$foo = foo(month: 12, ...);
 
 $foo(year: 2025);
 
diff --git a/Zend/tests/partial_application/fuzz_007.phpt b/Zend/tests/partial_application/fuzz_007.phpt
@@ -6,7 +6,7 @@ Closure application fuzz 007
 function foo(int $day = 1, int $month = UNDEFINED, int $year = 2005) {
 }
 
-$foo = foo(..., year: 2006);
+$foo = foo(year: 2006, ...);
 
 try {
     $foo(2);
diff --git a/Zend/tests/partial_application/named_placeholder_001.phpt b/Zend/tests/partial_application/named_placeholder_001.phpt
@@ -34,7 +34,7 @@ function bar($a = 1, $b = 2, ...$c) {
     var_dump($a, $b, $c);
 }
 
-$bar = bar(..., b: ?);
+$bar = bar(b: ?, ...);
 
 echo (string) new ReflectionFunction($bar);
 
@@ -47,7 +47,7 @@ try {
 }
 
 try {
-    $bar = $bar(..., c: ?);
+    $bar = $bar(c: ?, ...);
 } catch (\Throwable $e) {
     echo $e->getMessage(), "\n";
 }
diff --git a/Zend/tests/partial_application/pipe_optimization_008.phpt b/Zend/tests/partial_application/pipe_optimization_008.phpt
@@ -16,7 +16,7 @@ if (time() > 0) {
 }
 
 try {
-    2 |> foo(..., a: 1);
+    2 |> foo(a: 1, ...);
 } catch (\Throwable $e) {
     echo $e->getMessage(), "\n";
 }
@@ -32,9 +32,9 @@ $_main:
 0002 T1 = IS_SMALLER int(0) V2
 0003 JMPZ T1 0005
 0004 DECLARE_FUNCTION string("foo") 0
-0005 INIT_FCALL_BY_NAME 1 string("foo")
-0006 SEND_PLACEHOLDER
-0007 SEND_VAL_EX int(1) string("a")
+0005 INIT_FCALL_BY_NAME 0 string("foo")
+0006 SEND_VAL_EX int(1) string("a")
+0007 SEND_PLACEHOLDER
 0008 CHECK_PARTIAL_ARGS
 0009 T1 = CALLABLE_CONVERT_PARTIAL %d
 0010 INIT_DYNAMIC_CALL 1 T1
diff --git a/Zend/tests/partial_application/rfc_examples_001.phpt b/Zend/tests/partial_application/rfc_examples_001.phpt
@@ -58,19 +58,19 @@ $tests = [
         fn(int $i, string $s): array => stuff($i, $s, 3.5, $point),
     ],
     'The ... "everything else" placeholder respects named arguments' => [
-        stuff(?, ?, ..., f: 3.5, p: $point),
+        stuff(?, ?, f: 3.5, p: $point, ...),
         fn(int $i, string $s, int $m = 0): array => stuff($i, $s, 3.5, $point, $m),
     ],
     'Prefill all parameters, making a "delayed call" or "thunk"' => [
         stuff(1, 'hi', 3.4, $point, 5, ...),
         fn(): array => stuff(1, 'hi', 3.4, $point, 5),
     ],
-    'Placeholders may be named, too.  Their order doesn\'t matter as long as they come after the ..., if any' => [
+    'Placeholders may be named, too.  Their order doesn\'t matter as long as they come before the ..., if any' => [
         stuff(?, p: $point, f: ?, s: ?, m: 4),
         fn(int $i, string $s, float $f): array => stuff($i, $s, $f, $point, 4),
     ],
-    'Placeholders may be named, too.  Their order doesn\'t matter as long as they come after the ..., if any (2)' => [
-        stuff(..., m: 4, p: $point, i: ?),
+    'Placeholders may be named, too.  Their order doesn\'t matter as long as they come before the ..., if any (2)' => [
+        stuff(m: 4, p: $point, i: ?, ...),
         fn(int $i, string $s, float $f): array => stuff($i, $s, $f, $point, 4),
     ],
 ];
@@ -148,5 +148,5 @@ foreach ($tests as $test => [$pfa, $closure]) {
 # Named arguments can be pulled "out of order", and still work (2)
 # The ... "everything else" placeholder respects named arguments
 # Prefill all parameters, making a "delayed call" or "thunk"
-# Placeholders may be named, too.  Their order doesn't matter as long as they come after the ..., if any
-# Placeholders may be named, too.  Their order doesn't matter as long as they come after the ..., if any (2)
+# Placeholders may be named, too.  Their order doesn't matter as long as they come before the ..., if any
+# Placeholders may be named, too.  Their order doesn't matter as long as they come before the ..., if any (2)
diff --git a/Zend/tests/partial_application/variation_debug_002.phpt b/Zend/tests/partial_application/variation_debug_002.phpt
@@ -2,7 +2,7 @@
 Closure application variation debug internal
 --FILE--
 <?php
-var_dump(array_map(?, [1,2,3], [4,5, 6], ..., four: new stdClass));
+var_dump(array_map(?, [1,2,3], [4,5, 6], four: new stdClass, ...));
 ?>
 --EXPECTF--
 object(Closure)#%d (5) {
diff --git a/Zend/tests/partial_application/variation_invoke_001.phpt b/Zend/tests/partial_application/variation_invoke_001.phpt
@@ -6,7 +6,7 @@ function foo($a, $b) {
     return $a + $b;
 }
 
-$foo = foo(..., b: 10);
+$foo = foo(b: 10, ...);
 
 if ($foo->__invoke(32) == 42) {
     echo "OK\n";
diff --git a/Zend/tests/partial_application/variation_variadics_004.phpt b/Zend/tests/partial_application/variation_variadics_004.phpt
@@ -6,13 +6,13 @@ function foo(int $day = 1, int $month = 1, int $year = 2005) {
     return ["day" => $day, "month" => $month, "year" => $year];
 }
 
-$foo = foo(..., year: 2006);
+$foo = foo(year: 2006, ...);
 
 var_dump($foo(2));
 
-$foo = foo(..., month: 12);
+$foo = foo(month: 12, ...);
 
-$bar = $foo(..., year: 2016);
+$bar = $foo(year: 2016, ...);
 
 var_dump($foo(2));
 
diff --git a/Zend/tests/partial_application/variation_variadics_006.phpt b/Zend/tests/partial_application/variation_variadics_006.phpt
@@ -6,7 +6,7 @@ function foo($a) {
     var_dump(func_get_args());
 }
 
-$foo = foo(..., a: "a");
+$foo = foo(a: "a", ...);
 
 $foo();
 ?>
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -3790,13 +3790,20 @@ static uint32_t zend_compile_args(
 				may_have_undef = true;
 				*may_have_extra_named_args = true;
 			}
+
+			if (uses_variadic_placeholder) {
+				zend_error_noreturn(E_COMPILE_ERROR,
+					"Variadic placeholder must be last");
+			}
 		} else {
 			if (uses_arg_unpack) {
 				zend_error_noreturn(E_COMPILE_ERROR,
 					"Cannot use positional argument after argument unpacking");
 			}
 
-			if (uses_named_args) {
+			if (uses_named_args
+					&& (arg->kind != ZEND_AST_PLACEHOLDER_ARG
+						|| arg->attr != _IS_PLACEHOLDER_VARIADIC)) {
 				zend_error_noreturn(E_COMPILE_ERROR,
 					"Cannot use positional argument after named argument");
 			}
@@ -3808,11 +3815,14 @@ static uint32_t zend_compile_args(
 						"Variadic placeholder may only appear once");
 				} else {
 					zend_error_noreturn(E_COMPILE_ERROR,
-						"Only named arguments may follow variadic placeholder");
+						"Variadic placeholder must be last");
 				}
 			}
 
-			arg_count++;
+			if (arg->kind != ZEND_AST_PLACEHOLDER_ARG
+					|| arg->attr != _IS_PLACEHOLDER_VARIADIC) {
+				arg_count++;
+			}
 		}
 
 		if (arg->kind == ZEND_AST_PLACEHOLDER_ARG) {
@@ -3834,7 +3844,7 @@ static uint32_t zend_compile_args(
 				zend_string_addref(arg_name);
 				opline->op2.constant = zend_add_literal_string(&arg_name);
 				opline->result.num = zend_alloc_cache_slots(2);
-			} else {
+			} else if (arg->attr != _IS_PLACEHOLDER_VARIADIC) {
 				opline->op2.opline_num = arg_num;
 				opline->result.var = EX_NUM_TO_VAR(arg_num - 1);
 			}
@@ -6692,20 +6702,12 @@ static zend_ast *zend_partial_apply(zend_ast *callable_ast, zend_ast *pipe_arg)
 	zend_ast_list *arg_list = zend_ast_get_list(((zend_ast_fcc*)args_ast)->args);
 
 	zend_ast *first_placeholder = NULL;
-	bool uses_variadic_placeholder = false;
+	bool uses_named_args = false;
 
 	for (uint32_t i = 0; i < arg_list->children; i++) {
 		zend_ast *arg = arg_list->child[i];
 		if (arg->kind == ZEND_AST_NAMED_ARG) {
-			if (uses_variadic_placeholder) {
-				/* PFAs with both a variadic placeholder and named args can not
-				 * be optimized because the named arg may resolve to the
-				 * position of the placeholder: f(..., name: $v).
-				 * Arg placeholders ('?') are safe, as named args are not
-				 * allowed to override them. */
-				return NULL;
-			}
-
+			uses_named_args = true;
 			arg = arg->child[1];
 		}
 
@@ -6718,8 +6720,13 @@ static zend_ast *zend_partial_apply(zend_ast *callable_ast, zend_ast *pipe_arg)
 				 * so we don't optimize those. */
 				return NULL;
 			}
-			if (arg->attr == _IS_PLACEHOLDER_VARIADIC) {
-				uses_variadic_placeholder = true;
+			if (arg->attr == _IS_PLACEHOLDER_VARIADIC && uses_named_args) {
+				/* PFAs with both a variadic placeholder and named args can not
+				 * be optimized because the named arg may resolve to the
+				 * position of the placeholder: f(..., name: $v).
+				 * Arg placeholders ('?') are safe, as named args are not
+				 * allowed to override them. */
+				return NULL;
 			}
 		}
 	}
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -5576,10 +5576,6 @@ zval * ZEND_FASTCALL zend_handle_named_arg(
 	} else {
 		arg = ZEND_CALL_VAR_NUM(call, arg_offset);
 
-		if (UNEXPECTED(Z_TYPE_P(arg) == _IS_PLACEHOLDER_VARIADIC)) {
-			ZVAL_UNDEF(arg);
-		}
-
 		if (UNEXPECTED(!Z_ISUNDEF_P(arg))) {
 			zend_throw_error(NULL, "Named parameter $%s overwrites previous %s",
 				ZSTR_VAL(arg_name), Z_TYPE_P(arg) == _IS_PLACEHOLDER_ARG ? "placeholder" : "argument");
diff --git a/Zend/zend_partial.c b/Zend/zend_partial.c
@@ -495,15 +495,6 @@ zend_op_array *zp_compile(zval *this_ptr, zend_function *function,
 	/* Z_EXTRA(ZEND_CALL_ARG(call, 1)) is set in ZEND_SEND_PLACEHOLDER */
 	if (Z_EXTRA(argv[0]) == _IS_PLACEHOLDER_VARIADIC) {
 		variadic_partial = true;
-		if (Z_IS_PLACEHOLDER_VARIADIC_P(&argv[argc-1])
-				&& argc > function->common.num_args) {
-			/* A variadic placeholder at this position is only used to mark the
-			 * PFA variadic. We have taken note that the PFA is variadic.
-			 * The placeholder stands for the variadoc placeholder. No other
-			 * placeholders can follow. */
-			argc--;
-			new_argc--;
-		}
 		new_argc = MAX(new_argc, function->common.num_args);
 	}
 
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
@@ -9255,15 +9255,15 @@ ZEND_VM_HANDLER(211, ZEND_SEND_PLACEHOLDER, UNUSED, CONST|UNUSED)
 		if (UNEXPECTED(!arg)) {
 			HANDLE_EXCEPTION();
 		}
+		ZEND_ASSERT(opline->op1.num == _IS_PLACEHOLDER_ARG);
+		Z_TYPE_INFO_P(arg) = _IS_PLACEHOLDER_ARG;
+		Z_EXTRA_P(arg) = 0;
+	} else if (opline->op1.num == _IS_PLACEHOLDER_VARIADIC) {
+		Z_EXTRA_P(ZEND_CALL_ARG(call, 1)) = _IS_PLACEHOLDER_VARIADIC;
 	} else {
+		ZEND_ASSERT(opline->op1.num == _IS_PLACEHOLDER_ARG);
 		arg = ZEND_CALL_VAR(EX(call), opline->result.var);
-	}
-
-	Z_TYPE_INFO_P(arg) = opline->op1.num;
-
-	if (Z_TYPE_INFO_P(arg) == _IS_PLACEHOLDER_VARIADIC) {
-		Z_EXTRA_P(ZEND_CALL_ARG(call, 1)) = _IS_PLACEHOLDER_VARIADIC;
-	} else if (opline->op2.num == 1) {
+		Z_TYPE_INFO_P(arg) = _IS_PLACEHOLDER_ARG;
 		Z_EXTRA_P(arg) = 0;
 	}
 
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h

Original file line number	Diff line number	Diff line change
`@@ -151,6 +151,9 @@ ZEND_API void zend_analyze_calls(zend_arena *arena, zend_script script, uint32`
`151`	`151`	`call_info->named_args = true;`
`152`	`152`	`break;`
`153`	`153`	`}`
	`154`	`+ if (opline->opcode == ZEND_SEND_PLACEHOLDER && opline->op1.num == _IS_PLACEHOLDER_VARIADIC) {`
	`155`	`+ break;`
	`156`	`+ }`
`154`	`157`
`155`	`158`	`uint32_t num = opline->op2.num;`
`156`	`159`	`if (num > 0) {`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ function foo($a, $b) {`
`6`	`6`	`return $a + $b;`
`7`	`7`	`}`
`8`	`8`
`9`		`-$foo = foo(..., b: 10);`
	`9`	`+$foo = foo(b: 10, ...);`
`10`	`10`
`11`	`11`	`try {`
`12`	`12`	`$foo->__invoke(UNDEFINED);`
Original file line number	Diff line number	Diff line change
`@@ -6,9 +6,9 @@ Closure application fuzz 005`
`6`	`6`	`function foo(int $day = 1, int $month = 1, int $yearh = 1, int $year = 2005) {`
`7`	`7`	`}`
`8`	`8`
`9`		`-$foo = foo(..., month: 12);`
	`9`	`+$foo = foo(month: 12, ...);`
`10`	`10`
`11`		`-$bar = $foo(..., year: 2016);`
	`11`	`+$bar = $foo(year: 2016, ...);`
`12`	`12`
`13`	`13`	`?>`
`14`	`14`	`==DONE==`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ function foo(int $day = 1, int $month = 1, int $year = 2005) {`
`7`	`7`	`var_dump($day);`
`8`	`8`	`}`
`9`	`9`
`10`		`-$foo = foo(..., month: 12);`
	`10`	`+$foo = foo(month: 12, ...);`
`11`	`11`
`12`	`12`	`$foo(year: 2025);`
`13`	`13`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ Closure application fuzz 007`
`6`	`6`	`function foo(int $day = 1, int $month = UNDEFINED, int $year = 2005) {`
`7`	`7`	`}`
`8`	`8`
`9`		`-$foo = foo(..., year: 2006);`
	`9`	`+$foo = foo(year: 2006, ...);`
`10`	`10`
`11`	`11`	`try {`
`12`	`12`	`$foo(2);`