Unify magic method visibility check

This was missing entirely for the internal function case.

diff --git a/Zend/tests/magic_methods_007.phpt b/Zend/tests/magic_methods_007.phpt
index 86642b4ecb7247c84b61355f43568427f09ffc3a..5e0379060b4caef23629cbdf9c5dfb6acb589c23 100644 (file)
--- a/Zend/tests/magic_methods_007.phpt
+++ b/Zend/tests/magic_methods_007.phpt
@@ -9,6 +9,4 @@ abstract class b {
 
 ?>
 --EXPECTF--
-Warning: The magic method b::__set() must have public visibility in %s on line %d
-
 Fatal error: Method b::__set() must take exactly 2 arguments in %s on line %d

diff --git a/Zend/tests/magic_methods_010.phpt b/Zend/tests/magic_methods_010.phpt
index fda1cc1e10a2226470c88c24839e216fb13d57e0..3562189c344a30fb52f05364eb1b9380771f90e1 100644 (file)
--- a/Zend/tests/magic_methods_010.phpt
+++ b/Zend/tests/magic_methods_010.phpt
@@ -10,6 +10,4 @@ class a {
 
 ?>
 --EXPECTF--
-Warning: The magic method a::__toString() must have public visibility in %s on line %d
-
 Fatal error: Method a::__toString() cannot take arguments in %s on line %d

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 734c792fd5999141a8100c4c7c90b12a20b399d2..14f7cd695a8a05998cf925a622aa7d62290d6186 100644 (file)
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -2049,6 +2049,16 @@ static void zend_check_magic_method_static(
        }
 }
 
+static void zend_check_magic_method_public(
+               const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type)
+{
+       // TODO: Remove this warning after adding proper visibility handling.
+       if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) {
+               zend_error(E_WARNING, "The magic method %s::%s() must have public visibility",
+                       ZSTR_VAL(ce->name), name);
+       }
+}
+
 static void zend_check_magic_method_no_return_type(
                const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type)
 {
@@ -2079,38 +2089,50 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce,
        } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) {
                zend_check_magic_method_args(1, "__get", ce, fptr, error_type);
                zend_check_magic_method_non_static("__get", ce, fptr, error_type);
+               zend_check_magic_method_public("__get", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) {
                zend_check_magic_method_args(2, "__set", ce, fptr, error_type);
                zend_check_magic_method_non_static("__set", ce, fptr, error_type);
+               zend_check_magic_method_public("__set", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) {
                zend_check_magic_method_args(1, "__unset", ce, fptr, error_type);
                zend_check_magic_method_non_static("__unset", ce, fptr, error_type);
+               zend_check_magic_method_public("__unset", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) {
                zend_check_magic_method_args(1, "__isset", ce, fptr, error_type);
                zend_check_magic_method_non_static("__isset", ce, fptr, error_type);
+               zend_check_magic_method_public("__isset", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) {
                zend_check_magic_method_args(2, "__call", ce, fptr, error_type);
                zend_check_magic_method_non_static("__call", ce, fptr, error_type);
+               zend_check_magic_method_public("__call", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) {
                zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type);
                zend_check_magic_method_static("__callStatic", ce, fptr, error_type);
+               zend_check_magic_method_public("__callStatic", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) {
                zend_check_magic_method_args(0, "__toString", ce, fptr, error_type);
                zend_check_magic_method_non_static("__toString", ce, fptr, error_type);
+               zend_check_magic_method_public("__toString", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
                zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type);
                zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type);
+               zend_check_magic_method_public("__debugInfo", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, "__serialize")) {
                zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type);
                zend_check_magic_method_non_static("__serialize", ce, fptr, error_type);
+               zend_check_magic_method_public("__serialize", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, "__unserialize")) {
                zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type);
                zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type);
+               zend_check_magic_method_public("__unserialize", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, "__set_state")) {
                zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type);
                zend_check_magic_method_static("__set_state", ce, fptr, error_type);
+               zend_check_magic_method_public("__set_state", ce, fptr, error_type);
        } else if (zend_string_equals_literal(lcname, "__invoke")) {
                zend_check_magic_method_non_static("__invoke", ce, fptr, error_type);
+               zend_check_magic_method_public("__invoke", ce, fptr, error_type);
        }
 }
 /* }}} */
@@ -2294,11 +2316,9 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio
                                reg_function = NULL;
                        } else if (zend_string_equals_literal(lowercase_name, ZEND_CONSTRUCTOR_FUNC_NAME)) {
                                ctor = reg_function;
+                               ctor->common.fn_flags |= ZEND_ACC_CTOR;
                        } else if (zend_string_equals_literal(lowercase_name, ZEND_DESTRUCTOR_FUNC_NAME)) {
                                dtor = reg_function;
-                               if (internal_function->num_args) {
-                                       zend_error(error_type, "Destructor %s::%s() cannot take arguments", ZSTR_VAL(scope->name), ptr->fname);
-                               }
                        } else if (zend_string_equals_literal(lowercase_name, ZEND_CLONE_FUNC_NAME)) {
                                clone = reg_function;
                        } else if (zend_string_equals_literal(lowercase_name, ZEND_CALL_FUNC_NAME)) {
@@ -2368,9 +2388,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio
                scope->__debugInfo = __debugInfo;
                scope->__serialize = __serialize;
                scope->__unserialize = __unserialize;
-               if (ctor) {
-                       ctor->common.fn_flags |= ZEND_ACC_CTOR;
-               }
                efree((char*)lc_class_name);
        }
        return SUCCESS;

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index c2d0a117f23ef125c7026b2f0e6d38a50964ef59..cabc1087a7fcc5db4f34f034167eb0e5e1ffdd70 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -6443,16 +6443,6 @@ static void zend_compile_implicit_closure_uses(closure_info *info)
        ZEND_HASH_FOREACH_END();
 }
 
-static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method) /* {{{ */
-{
-       if (!(attr & ZEND_ACC_PUBLIC)) {
-               zend_error(E_WARNING,
-                               "The magic method %s::%s() must have public visibility",
-                               ZSTR_VAL(ce->name), method);
-       }
-}
-/* }}} */
-
 static void add_stringable_interface(zend_class_entry *ce) {
        for (uint32_t i = 0; i < ce->num_interfaces; i++) {
                if (zend_string_equals_literal(ce->interface_names[i].lc_name, "stringable")) {
@@ -6523,49 +6513,36 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name,
                /* pass */
        } else if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) {
                ce->constructor = (zend_function *) op_array;
+               ce->constructor->common.fn_flags |= ZEND_ACC_CTOR;
        } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) {
                ce->destructor = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) {
                ce->clone = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__call");
                ce->__call = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__callStatic");
                ce->__callstatic = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__get");
                ce->__get = (zend_function *) op_array;
                ce->ce_flags |= ZEND_ACC_USE_GUARDS;
        } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__set");
                ce->__set = (zend_function *) op_array;
                ce->ce_flags |= ZEND_ACC_USE_GUARDS;
        } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__unset");
                ce->__unset = (zend_function *) op_array;
                ce->ce_flags |= ZEND_ACC_USE_GUARDS;
        } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__isset");
                ce->__isset = (zend_function *) op_array;
                ce->ce_flags |= ZEND_ACC_USE_GUARDS;
        } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__toString");
                ce->__tostring = (zend_function *) op_array;
                add_stringable_interface(ce);
-       } else if (zend_string_equals_literal(lcname, ZEND_INVOKE_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__invoke");
        } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
-               zend_check_magic_method_attr(fn_flags, ce, "__debugInfo");
                ce->__debugInfo = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, "__serialize")) {
-               zend_check_magic_method_attr(fn_flags, ce, "__serialize");
                ce->__serialize = (zend_function *) op_array;
        } else if (zend_string_equals_literal(lcname, "__unserialize")) {
-               zend_check_magic_method_attr(fn_flags, ce, "__unserialize");
                ce->__unserialize = (zend_function *) op_array;
-       } else if (zend_string_equals_literal(lcname, "__set_state")) {
-        zend_check_magic_method_attr(fn_flags, ce, "__set_state");
        }
 
        return lcname;
@@ -6740,6 +6717,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, zend_bool toplevel) /*
        zend_compile_stmt(stmt_ast);
 
        if (is_method) {
+               CG(zend_lineno) = decl->start_lineno;
                zend_check_magic_method_implementation(
                        CG(active_class_entry), (zend_function *) op_array, method_lcname, E_COMPILE_ERROR);
                zend_string_release_ex(method_lcname, 0);
@@ -7159,10 +7137,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) /
        /* Reset lineno for final opcodes and errors */
        CG(zend_lineno) = ast->lineno;
 
-       if (ce->constructor) {
-               ce->constructor->common.fn_flags |= ZEND_ACC_CTOR;
-       }
-
        if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) {
                zend_verify_abstract_class(ce);
        }