Improvements in modifier parsing (#9926)

Use a shared non-terminal for all class modifiers. This avoids conflicts when
adding modifiers that are only valid for certain targets. This change is
necessary for asymmetric visibility but might be useful for other future
additions.

Closes GH-9926

Zend/tests/access_modifiers_007.phpt

@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Cannot use the final modifier on an abstract class member in %s on line %d
+Fatal error: Cannot use the final modifier on an abstract method in %s on line %d

Zend/tests/ctor_promotion_additional_modifiers.phpt

@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Parse error: syntax error, unexpected token "static", expecting variable in %s on line %d
+Fatal error: Cannot use the static modifier on a promoted property in %s on line %d

Zend/tests/errmsg_037.phpt

@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Properties cannot be declared abstract in %s on line %d
+Fatal error: Cannot use the abstract modifier on a property in %s on line %d

Zend/tests/errmsg_038.phpt

@@ -10,4 +10,4 @@ class test {
 echo "Done\n";
 ?>
 --EXPECTF--
-Fatal error: Cannot declare property test::$var final, the final modifier is allowed only for methods, classes, and class constants in %s on line %d
+Fatal error: Cannot use the final modifier on a property in %s on line %d

Zend/tests/readonly_props/readonly_const.phpt

@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as constant modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a class constant in %s on line %d

Zend/tests/readonly_props/readonly_method.phpt

@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as method modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a method in %s on line %d

Zend/tests/readonly_props/readonly_method_trait.phpt

@@ -9,4 +9,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'readonly' as method modifier in %s on line %d
+Fatal error: Cannot use the readonly modifier on a method in %s on line %d

Zend/tests/traits/error_013.phpt

@@ -16,4 +16,4 @@ var_dump($x->test());
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'static' as method modifier in %s on line %d
+Fatal error: Cannot use "static" as method modifier in trait alias in %s on line %d

Zend/tests/traits/language018.phpt

@@ -12,4 +12,4 @@ class C1 {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'abstract' as method modifier in %s on line %d
+Fatal error: Cannot use "abstract" as method modifier in trait alias in %s on line %d

Zend/tests/traits/language019.phpt

@@ -12,4 +12,4 @@ class C1 {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'final' as method modifier in %s on line %d
+Fatal error: Cannot use "final" as method modifier in trait alias in %s on line %d

Zend/tests/type_declarations/static_type_param.phpt

@@ -12,4 +12,4 @@ class Test {
 
 ?>
 --EXPECTF--
-Parse error: syntax error, unexpected token "static", expecting variable in %s on line %d
+Fatal error: Cannot use the static modifier on a promoted property in %s on line %d

Zend/zend_ast.h

@@ -66,6 +66,7 @@ enum _zend_ast_kind {
 	ZEND_AST_ATTRIBUTE_LIST,
 	ZEND_AST_ATTRIBUTE_GROUP,
 	ZEND_AST_MATCH_ARM_LIST,
+	ZEND_AST_MODIFIER_LIST,
 
 	/* 0 child nodes */
 	ZEND_AST_MAGIC_CONST = 0 << ZEND_AST_NUM_CHILDREN_SHIFT,

Zend/zend_compile.c

@@ -787,6 +787,96 @@ static void zend_do_free(znode *op1) /* {{{ */
 }
 /* }}} */
 
+
+static char *zend_modifier_token_to_string(uint32_t token)
+{
+	switch (token) {
+		case T_PUBLIC:
+			return "public";
+		case T_PROTECTED:
+			return "protected";
+		case T_PRIVATE:
+			return "private";
+		case T_STATIC:
+			return "static";
+		case T_FINAL:
+			return "final";
+		case T_READONLY:
+			return "readonly";
+		case T_ABSTRACT:
+			return "abstract";
+		EMPTY_SWITCH_DEFAULT_CASE()
+	}
+}
+
+uint32_t zend_modifier_token_to_flag(zend_modifier_target target, uint32_t token)
+{
+	switch (token) {
+		case T_PUBLIC:
+			return ZEND_ACC_PUBLIC;
+		case T_PROTECTED:
+			return ZEND_ACC_PROTECTED;
+		case T_PRIVATE:
+			return ZEND_ACC_PRIVATE;
+		case T_READONLY:
+			if (target == ZEND_MODIFIER_TARGET_PROPERTY || target == ZEND_MODIFIER_TARGET_CPP) {
+				return ZEND_ACC_READONLY;
+			}
+			break;
+		case T_ABSTRACT:
+			if (target == ZEND_MODIFIER_TARGET_METHOD) {
+				return ZEND_ACC_ABSTRACT;
+			}
+			break;
+		case T_FINAL:
+			if (target == ZEND_MODIFIER_TARGET_METHOD || target == ZEND_MODIFIER_TARGET_CONSTANT) {
+				return ZEND_ACC_FINAL;
+			}
+			break;
+		case T_STATIC:
+			if (target == ZEND_MODIFIER_TARGET_PROPERTY || target == ZEND_MODIFIER_TARGET_METHOD) {
+				return ZEND_ACC_STATIC;
+			}
+			break;
+	}
+
+	char *member;
+	if (target == ZEND_MODIFIER_TARGET_PROPERTY) {
+		member = "property";
+	} else if (target == ZEND_MODIFIER_TARGET_METHOD) {
+		member = "method";
+	} else if (target == ZEND_MODIFIER_TARGET_CONSTANT) {
+		member = "class constant";
+	} else if (target == ZEND_MODIFIER_TARGET_CPP) {
+		member = "promoted property";
+	} else {
+		ZEND_UNREACHABLE();
+	}
+
+	zend_throw_exception_ex(zend_ce_compile_error, 0,
+		"Cannot use the %s modifier on a %s", zend_modifier_token_to_string(token), member);
+	return 0;
+}
+
+uint32_t zend_modifier_list_to_flags(zend_modifier_target target, zend_ast *modifiers)
+{
+	uint32_t flags = 0;
+	zend_ast_list *modifier_list = zend_ast_get_list(modifiers);
+
+	for (uint32_t i = 0; i < modifier_list->children; i++) {
+		uint32_t new_flag = zend_modifier_token_to_flag(target, (uint32_t) Z_LVAL_P(zend_ast_get_zval(modifier_list->child[i])));
+		if (!new_flag) {
+			return 0;
+		}
+		flags = zend_add_member_modifier(flags, new_flag, target);
+		if (!flags) {
+			return 0;
+		}
+	}
+
+	return flags;
+}
+
 uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
@@ -812,7 +902,7 @@ uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 }
 /* }}} */
 
-uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
+uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag, zend_modifier_target target) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
 	if ((flags & ZEND_ACC_PPP_MASK) && (new_flag & ZEND_ACC_PPP_MASK)) {
@@ -837,9 +927,9 @@ uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 			"Multiple readonly modifiers are not allowed", 0);
 		return 0;
 	}
-	if ((new_flags & ZEND_ACC_ABSTRACT) && (new_flags & ZEND_ACC_FINAL)) {
+	if (target == ZEND_MODIFIER_TARGET_METHOD && (new_flags & ZEND_ACC_ABSTRACT) && (new_flags & ZEND_ACC_FINAL)) {
 		zend_throw_exception(zend_ce_compile_error,
-			"Cannot use the final modifier on an abstract class member", 0);
+			"Cannot use the final modifier on an abstract method", 0);
 		return 0;
 	}
 	return new_flags;
@@ -7474,10 +7564,6 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f
 		zend_error_noreturn(E_COMPILE_ERROR, "Enum %s cannot include properties", ZSTR_VAL(ce->name));
 	}
 
-	if (flags & ZEND_ACC_ABSTRACT) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Properties cannot be declared abstract");
-	}
-
 	for (i = 0; i < children; ++i) {
 		zend_property_info *info;
 		zend_ast *prop_ast = list->child[i];
@@ -7505,12 +7591,6 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f
 			doc_comment = zend_string_copy(zend_ast_get_str(doc_comment_ast));
 		}
 
-		if (flags & ZEND_ACC_FINAL) {
-			zend_error_noreturn(E_COMPILE_ERROR, "Cannot declare property %s::$%s final, "
-				"the final modifier is allowed only for methods, classes, and class constants",
-				ZSTR_VAL(ce->name), ZSTR_VAL(name));
-		}
-
 		if (zend_hash_exists(&ce->properties_info, name)) {
 			zend_error_noreturn(E_COMPILE_ERROR, "Cannot redeclare %s::$%s",
 				ZSTR_VAL(ce->name), ZSTR_VAL(name));
@@ -7583,16 +7663,14 @@ static void zend_compile_prop_group(zend_ast *ast) /* {{{ */
 }
 /* }}} */
 
-static void zend_check_const_and_trait_alias_attr(uint32_t attr, const char* entity) /* {{{ */
+static void zend_check_trait_alias_modifiers(uint32_t attr) /* {{{ */
 {
 	if (attr & ZEND_ACC_STATIC) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'static' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"static\" as method modifier in trait alias");
 	} else if (attr & ZEND_ACC_ABSTRACT) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'abstract' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"abstract\" as method modifier in trait alias");
 	} else if (attr & ZEND_ACC_FINAL) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'final' as %s modifier", entity);
-	} else if (attr & ZEND_ACC_READONLY) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'readonly' as %s modifier", entity);
+		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"final\" as method modifier in trait alias");
 	}
 }
 /* }}} */
@@ -7613,10 +7691,6 @@ static void zend_compile_class_const_decl(zend_ast *ast, uint32_t flags, zend_as
 		zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL;
 		zval value_zv;
 
-		if (UNEXPECTED(flags & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT|ZEND_ACC_READONLY))) {
-			zend_check_const_and_trait_alias_attr(flags, "constant");
-		}
-
 		if (UNEXPECTED((flags & ZEND_ACC_PRIVATE) && (flags & ZEND_ACC_FINAL))) {
 			zend_error_noreturn(
 				E_COMPILE_ERROR, "Private constant %s::%s cannot be final as it is not visible to other classes",
@@ -7687,7 +7761,7 @@ static void zend_compile_trait_alias(zend_ast *ast) /* {{{ */
 
 	zend_trait_alias *alias;
 
-	zend_check_const_and_trait_alias_attr(modifiers, "method");
+	zend_check_trait_alias_modifiers(modifiers);
 
 	alias = emalloc(sizeof(zend_trait_alias));
 	zend_compile_method_ref(method_ref_ast, &alias->trait_method);

Zend/zend_compile.h

@@ -806,11 +806,22 @@ ZEND_API binary_op_type get_binary_op(int opcode);
 void zend_stop_lexing(void);
 void zend_emit_final_return(bool return_one);
 
+typedef enum {
+	ZEND_MODIFIER_TARGET_PROPERTY = 0,
+	ZEND_MODIFIER_TARGET_METHOD,
+	ZEND_MODIFIER_TARGET_CONSTANT,
+	ZEND_MODIFIER_TARGET_CPP,
+} zend_modifier_target;
+
 /* Used during AST construction */
 zend_ast *zend_ast_append_str(zend_ast *left, zend_ast *right);
 zend_ast *zend_negate_num_string(zend_ast *ast);
 uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag);
-uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag);
+uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag, zend_modifier_target target);
+
+uint32_t zend_modifier_token_to_flag(zend_modifier_target target, uint32_t flags);
+uint32_t zend_modifier_list_to_flags(zend_modifier_target target, zend_ast *modifiers);
+
 bool zend_handle_encoding_declaration(zend_ast *ast);
 
 ZEND_API zend_class_entry *zend_bind_class_in_slot(

Zend/zend_language_parser.y

@@ -278,11 +278,10 @@ static YYSIZE_T zend_yytnamerr(char*, const char*);
 %type <ast> attribute_decl attribute attributes attribute_group namespace_declaration_name
 %type <ast> match match_arm_list non_empty_match_arm_list match_arm match_arm_cond_list
 %type <ast> enum_declaration_statement enum_backing_type enum_case enum_case_expr
-%type <ast> function_name
+%type <ast> function_name non_empty_member_modifiers
 
-%type <num> returns_ref function fn is_reference is_variadic variable_modifiers
-%type <num> method_modifiers non_empty_member_modifiers member_modifier
-%type <num> optional_property_modifiers property_modifier
+%type <num> returns_ref function fn is_reference is_variadic property_modifiers
+%type <num> method_modifiers class_const_modifiers member_modifier optional_cpp_modifiers
 %type <num> class_modifiers class_modifier use_type backup_fn_flags
 
 %type <ptr> backup_lex_pos
@@ -782,24 +781,20 @@ attributed_parameter:
 	|	parameter				{ $$ = $1; }
 ;
 
-optional_property_modifiers:
-		%empty					{ $$ = 0; }
-	|	optional_property_modifiers property_modifier
-			{ $$ = zend_add_member_modifier($1, $2); if (!$$) { YYERROR; } }
-
-property_modifier:
-		T_PUBLIC				{ $$ = ZEND_ACC_PUBLIC; }
-	|	T_PROTECTED				{ $$ = ZEND_ACC_PROTECTED; }
-	|	T_PRIVATE				{ $$ = ZEND_ACC_PRIVATE; }
-	|	T_READONLY				{ $$ = ZEND_ACC_READONLY; }
+optional_cpp_modifiers:
+		%empty
+			{ $$ = 0; }
+	|	non_empty_member_modifiers
+			{ $$ = zend_modifier_list_to_flags(ZEND_MODIFIER_TARGET_CPP, $1);
+			  if (!$$) { YYERROR; } }
 ;
 
 parameter:
-		optional_property_modifiers optional_type_without_static
+		optional_cpp_modifiers optional_type_without_static
 		is_reference is_variadic T_VARIABLE backup_doc_comment
 			{ $$ = zend_ast_create_ex(ZEND_AST_PARAM, $1 | $3 | $4, $2, $5, NULL,
 					NULL, $6 ? zend_ast_create_zval_from_str($6) : NULL); }
-	|	optional_property_modifiers optional_type_without_static
+	|	optional_cpp_modifiers optional_type_without_static
 		is_reference is_variadic T_VARIABLE backup_doc_comment '=' expr
 			{ $$ = zend_ast_create_ex(ZEND_AST_PARAM, $1 | $3 | $4, $2, $5, $8,
 					NULL, $6 ? zend_ast_create_zval_from_str($6) : NULL); }
@@ -930,10 +925,10 @@ class_statement_list:
 
 
 attributed_class_statement:
-		variable_modifiers optional_type_without_static property_list ';'
+		property_modifiers optional_type_without_static property_list ';'
 			{ $$ = zend_ast_create(ZEND_AST_PROP_GROUP, $2, $3, NULL);
 			  $$->attr = $1; }
-	|	method_modifiers T_CONST class_const_list ';'
+	|	class_const_modifiers T_CONST class_const_list ';'
 			{ $$ = zend_ast_create(ZEND_AST_CLASS_CONST_GROUP, $3, NULL);
 			  $$->attr = $1; }
 	|	method_modifiers function returns_ref identifier backup_doc_comment '(' parameter_list ')'
@@ -986,9 +981,15 @@ trait_alias:
 			  if (zend_lex_tstring(&zv, $3) == FAILURE) { YYABORT; }
 			  $$ = zend_ast_create(ZEND_AST_TRAIT_ALIAS, $1, zend_ast_create_zval(&zv)); }
 	|	trait_method_reference T_AS member_modifier identifier
-			{ $$ = zend_ast_create_ex(ZEND_AST_TRAIT_ALIAS, $3, $1, $4); }
+			{ uint32_t modifiers = zend_modifier_token_to_flag(ZEND_MODIFIER_TARGET_METHOD, $3);
+			  $$ = zend_ast_create_ex(ZEND_AST_TRAIT_ALIAS, modifiers, $1, $4);
+			  /* identifier nonterminal can cause allocations, so we need to free the node */
+			  if (!modifiers) { zend_ast_destroy($$); YYERROR; } }
 	|	trait_method_reference T_AS member_modifier
-			{ $$ = zend_ast_create_ex(ZEND_AST_TRAIT_ALIAS, $3, $1, NULL); }
+			{ uint32_t modifiers = zend_modifier_token_to_flag(ZEND_MODIFIER_TARGET_METHOD, $3);
+			  $$ = zend_ast_create_ex(ZEND_AST_TRAIT_ALIAS, modifiers, $1, NULL);
+			  /* identifier nonterminal can cause allocations, so we need to free the node */
+			  if (!modifiers) { zend_ast_destroy($$); YYERROR; } }
 ;
 
 trait_method_reference:
@@ -1007,31 +1008,47 @@ method_body:
 	|	'{' inner_statement_list '}'	{ $$ = $2; }
 ;
 
-variable_modifiers:
-		non_empty_member_modifiers		{ $$ = $1; }
-	|	T_VAR							{ $$ = ZEND_ACC_PUBLIC; }
+property_modifiers:
+		non_empty_member_modifiers
+			{ $$ = zend_modifier_list_to_flags(ZEND_MODIFIER_TARGET_PROPERTY, $1);
+			  if (!$$) { YYERROR; } }
+	|	T_VAR
+			{ $$ = ZEND_ACC_PUBLIC; }
 ;
 
 method_modifiers:
-		%empty						{ $$ = ZEND_ACC_PUBLIC; }
+		%empty
+			{ $$ = ZEND_ACC_PUBLIC; }
 	|	non_empty_member_modifiers
-			{ $$ = $1; if (!($$ & ZEND_ACC_PPP_MASK)) { $$ |= ZEND_ACC_PUBLIC; } }
+			{ $$ = zend_modifier_list_to_flags(ZEND_MODIFIER_TARGET_METHOD, $1);
+			  if (!$$) { YYERROR; }
+			  if (!($$ & ZEND_ACC_PPP_MASK)) { $$ |= ZEND_ACC_PUBLIC; } }
+;
+
+class_const_modifiers:
+		%empty
+			{ $$ = ZEND_ACC_PUBLIC; }
+	|	non_empty_member_modifiers
+			{ $$ = zend_modifier_list_to_flags(ZEND_MODIFIER_TARGET_CONSTANT, $1);
+			  if (!$$) { YYERROR; }
+			  if (!($$ & ZEND_ACC_PPP_MASK)) { $$ |= ZEND_ACC_PUBLIC; } }
 ;
 
 non_empty_member_modifiers:
-		member_modifier			{ $$ = $1; }
+		member_modifier
+			{ $$ = zend_ast_create_list(1, ZEND_AST_MODIFIER_LIST, zend_ast_create_zval_from_long($1)); }
 	|	non_empty_member_modifiers member_modifier
-			{ $$ = zend_add_member_modifier($1, $2); if (!$$) { YYERROR; } }
+			{ $$ = zend_ast_list_add($1, zend_ast_create_zval_from_long($2)); }
 ;
 
 member_modifier:
-		T_PUBLIC				{ $$ = ZEND_ACC_PUBLIC; }
-	|	T_PROTECTED				{ $$ = ZEND_ACC_PROTECTED; }
-	|	T_PRIVATE				{ $$ = ZEND_ACC_PRIVATE; }
-	|	T_STATIC				{ $$ = ZEND_ACC_STATIC; }
-	|	T_ABSTRACT				{ $$ = ZEND_ACC_ABSTRACT; }
-	|	T_FINAL					{ $$ = ZEND_ACC_FINAL; }
-	|	T_READONLY				{ $$ = ZEND_ACC_READONLY; }
+		T_PUBLIC				{ $$ = T_PUBLIC; }
+	|	T_PROTECTED				{ $$ = T_PROTECTED; }
+	|	T_PRIVATE				{ $$ = T_PRIVATE; }
+	|	T_STATIC				{ $$ = T_STATIC; }
+	|	T_ABSTRACT				{ $$ = T_ABSTRACT; }
+	|	T_FINAL					{ $$ = T_FINAL; }
+	|	T_READONLY				{ $$ = T_READONLY; }
 ;
 
 property_list:

ext/reflection/tests/bug74454.phpt

@@ -14,4 +14,4 @@ function load_file() {
 }
 ?>
 --EXPECT--
-ParseError: syntax error, unexpected token "if", expecting "function" or "const"
+ParseError: syntax error, unexpected token "if", expecting "function"

tests/classes/abstract_final.phpt

@@ -10,4 +10,4 @@ class fail {
 echo "Done\n"; // Shouldn't be displayed
 ?>
 --EXPECTF--
-Fatal error: Cannot use the final modifier on an abstract class member in %s on line %d
+Fatal error: Cannot use the final modifier on an abstract method in %s on line %d

tests/classes/constants_visibility_005.phpt

@@ -7,4 +7,4 @@ class A {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'static' as constant modifier in %s on line 3
+Fatal error: Cannot use the static modifier on a class constant in %s on line %d

tests/classes/constants_visibility_006.phpt

@@ -7,4 +7,4 @@ class A {
 }
 ?>
 --EXPECTF--
-Fatal error: Cannot use 'abstract' as constant modifier in %s on line 3
+Fatal error: Cannot use the abstract modifier on a class constant in %s on line %d

tests/classes/final_abstract.phpt

@@ -10,4 +10,4 @@ class fail {
 echo "Done\n"; // Shouldn't be displayed
 ?>
 --EXPECTF--
-Fatal error: Cannot use the final modifier on an abstract class member in %s
+Fatal error: Cannot use the final modifier on an abstract method in %s on line %d

tests/classes/interface_method_final.phpt

@@ -9,4 +9,4 @@ class if_a {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use the final modifier on an abstract class member in %s on line %d
+Fatal error: Cannot use the final modifier on an abstract method in %s on line %d