[prev in list] [next in list] [prev in thread] [next in thread]
List: php-cvs
Subject: [PHP-CVS] Re: [PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_2/Zend/tests/bug51421.phpt
From: Etienne Kneuss <webmaster () colder ! ch>
Date: 2010-06-28 10:39:50
Message-ID: AANLkTili1xjcCyw4EKEKwo4Mo6WIH-9HlpIccd6f8iBV () mail ! gmail ! com
[Download RAW message or body]
Hi,
2010/6/28 Johannes Schlüter <johannes@schlueters.de>
> Hi,
>
> On Sat, 2010-06-26 at 22:05 +0000, Felipe Pena wrote:
> > felipe Sat, 26 Jun 2010 22:05:13 +0000
> >
> > Revision: http://svn.php.net/viewvc?view=revision&revision=300770
> >
> > Log:
> > - Fixed bug #51421 (Abstract __construct constructor argument list not
> enforced)
> >
> > Bug: http://bugs.php.net/51421 (Closed) Abstract __construct constructor
> argument list not enforced
>
> Won't this break compatibility?
>
> I'd say the change is logically correct but not sure we should break BC
> there during RC.
>
At the risk of starting some political debate, I believe that the current
state of prototype checks make close to no sense in PHP, for 4 reasons:
1) arguments can be gathered dynamically in the function, using
func_get_args. For that reason, PHP gracefully allow a call with too many
arguments.
2) a child method should be allowed to define type hints in a contra-variant
manner (currently, it's invariant)
3) a child method should be allowed to return a ref even if the parent
method is not defined to do so. (returning a ref is an additional
constraint, and following co-variance of return values, it should be
allowed). The basic example of this annoyance is: abstract class A
implements ArrayAcces { public function &__offsetGet($name) { ... } }
4) all in all: it shouldn't throw a fatal error, those are not critical
situations for the engine.
I'd like to propose to relax such checks, by either allowing more (e.g. 1, 2
and 3) which can be painful and complicated, or simply removing those
checks.
Best,
>
> johannes
>
> > Changed paths:
> > A php/php-src/branches/PHP_5_2/Zend/tests/bug51421.phpt
> > U php/php-src/branches/PHP_5_2/Zend/zend_compile.c
> > A php/php-src/branches/PHP_5_3/Zend/tests/bug51421.phpt
> > U php/php-src/branches/PHP_5_3/Zend/zend_compile.c
> > A php/php-src/trunk/Zend/tests/bug51421.phpt
> > U php/php-src/trunk/Zend/zend_compile.c
> >
> > Added: php/php-src/branches/PHP_5_2/Zend/tests/bug51421.phpt
> > ===================================================================
> > --- php/php-src/branches/PHP_5_2/Zend/tests/bug51421.phpt
> (rev 0)
> > +++ php/php-src/branches/PHP_5_2/Zend/tests/bug51421.phpt 2010-06-26
> 22:05:13 UTC (rev 300770)
> > @@ -0,0 +1,18 @@
> > +--TEST--
> > +Bug #51421 (Abstract __construct constructor argument list not enforced)
> > +--FILE--
> > +<?php
> > +
> > +class ExampleClass {}
> > +
> > +abstract class TestInterface {
> > + abstract public function __construct(ExampleClass $var);
> > +}
> > +
> > +class Test extends TestInterface {
> > + public function __construct() {}
> > +}
> > +
> > +?>
> > +--EXPECTF--
> > +Fatal error: Declaration of Test::__construct() must be compatible with
> that of TestInterface::__construct() in %s on line %d
> >
> >
> > Property changes on:
> php/php-src/branches/PHP_5_2/Zend/tests/bug51421.phpt
> > ___________________________________________________________________
> > Added: svn:keywords
> > + Id Rev Revision
> > Added: svn:eol-style
> > + native
> >
> > Modified: php/php-src/branches/PHP_5_2/Zend/zend_compile.c
> > ===================================================================
> > --- php/php-src/branches/PHP_5_2/Zend/zend_compile.c 2010-06-26 21:29:56
> UTC (rev 300769)
> > +++ php/php-src/branches/PHP_5_2/Zend/zend_compile.c 2010-06-26 22:05:13
> UTC (rev 300770)
> > @@ -2009,13 +2009,20 @@
> > {
> > zend_uint i;
> >
> > - /* If it's a user function then arg_info == NULL means we don't
> have any parameters but we still need to do the arg number checks. We are
> only willing to ignore this for internal functions because extensions don't
> always define arg_info. */
> > + /* If it's a user function then arg_info == NULL means we don't
> have any parameters but
> > + * we still need to do the arg number checks. We are only willing
> to ignore this for internal
> > + * functions because extensions don't always define arg_info.
> > + */
> > if (!proto || (!proto->common.arg_info && proto->common.type !=
> ZEND_USER_FUNCTION)) {
> > return 1;
> > }
> >
> > - /* Checks for constructors only if they are declared in an
> interface */
> > - if ((fe->common.fn_flags & ZEND_ACC_CTOR) &&
> !(proto->common.scope->ce_flags & ZEND_ACC_INTERFACE)) {
> > + /* Checks for constructors only if they are declared in an
> interface,
> > + * or explicitly marked as abstract
> > + */
> > + if ((fe->common.fn_flags & ZEND_ACC_CTOR)
> > + && ((proto->common.scope->ce_flags & ZEND_ACC_INTERFACE) ==
> 0
> > + && (proto->common.fn_flags & ZEND_ACC_ABSTRACT) ==
> 0)) {
> > return 1;
> > }
> >
> >
> > Added: php/php-src/branches/PHP_5_3/Zend/tests/bug51421.phpt
> > ===================================================================
> > --- php/php-src/branches/PHP_5_3/Zend/tests/bug51421.phpt
> (rev 0)
> > +++ php/php-src/branches/PHP_5_3/Zend/tests/bug51421.phpt 2010-06-26
> 22:05:13 UTC (rev 300770)
> > @@ -0,0 +1,18 @@
> > +--TEST--
> > +Bug #51421 (Abstract __construct constructor argument list not enforced)
> > +--FILE--
> > +<?php
> > +
> > +class ExampleClass {}
> > +
> > +abstract class TestInterface {
> > + abstract public function __construct(ExampleClass $var);
> > +}
> > +
> > +class Test extends TestInterface {
> > + public function __construct() {}
> > +}
> > +
> > +?>
> > +--EXPECTF--
> > +Fatal error: Declaration of Test::__construct() must be compatible with
> that of TestInterface::__construct() in %s on line %d
> >
> >
> > Property changes on:
> php/php-src/branches/PHP_5_3/Zend/tests/bug51421.phpt
> > ___________________________________________________________________
> > Added: svn:keywords
> > + Id Rev Revision
> > Added: svn:eol-style
> > + native
> >
> > Modified: php/php-src/branches/PHP_5_3/Zend/zend_compile.c
> > ===================================================================
> > --- php/php-src/branches/PHP_5_3/Zend/zend_compile.c 2010-06-26 21:29:56
> UTC (rev 300769)
> > +++ php/php-src/branches/PHP_5_3/Zend/zend_compile.c 2010-06-26 22:05:13
> UTC (rev 300770)
> > @@ -2532,13 +2532,20 @@
> > {
> > zend_uint i;
> >
> > - /* If it's a user function then arg_info == NULL means we don't
> have any parameters but we still need to do the arg number checks. We are
> only willing to ignore this for internal functions because extensions don't
> always define arg_info. */
> > + /* If it's a user function then arg_info == NULL means we don't
> have any parameters but
> > + * we still need to do the arg number checks. We are only willing
> to ignore this for internal
> > + * functions because extensions don't always define arg_info.
> > + */
> > if (!proto || (!proto->common.arg_info && proto->common.type !=
> ZEND_USER_FUNCTION)) {
> > return 1;
> > }
> >
> > - /* Checks for constructors only if they are declared in an
> interface */
> > - if ((fe->common.fn_flags & ZEND_ACC_CTOR) &&
> !(proto->common.scope->ce_flags & ZEND_ACC_INTERFACE)) {
> > + /* Checks for constructors only if they are declared in an
> interface,
> > + * or explicitly marked as abstract
> > + */
> > + if ((fe->common.fn_flags & ZEND_ACC_CTOR)
> > + && ((proto->common.scope->ce_flags & ZEND_ACC_INTERFACE) ==
> 0
> > + && (proto->common.fn_flags & ZEND_ACC_ABSTRACT) ==
> 0)) {
> > return 1;
> > }
> >
> >
> > Added: php/php-src/trunk/Zend/tests/bug51421.phpt
> > ===================================================================
> > --- php/php-src/trunk/Zend/tests/bug51421.phpt
> (rev 0)
> > +++ php/php-src/trunk/Zend/tests/bug51421.phpt 2010-06-26 22:05:13
> UTC (rev 300770)
> > @@ -0,0 +1,18 @@
> > +--TEST--
> > +Bug #51421 (Abstract __construct constructor argument list not enforced)
> > +--FILE--
> > +<?php
> > +
> > +class ExampleClass {}
> > +
> > +abstract class TestInterface {
> > + abstract public function __construct(ExampleClass $var);
> > +}
> > +
> > +class Test extends TestInterface {
> > + public function __construct() {}
> > +}
> > +
> > +?>
> > +--EXPECTF--
> > +Fatal error: Declaration of Test::__construct() must be compatible with
> that of TestInterface::__construct() in %s on line %d
> >
> >
> > Property changes on: php/php-src/trunk/Zend/tests/bug51421.phpt
> > ___________________________________________________________________
> > Added: svn:keywords
> > + Id Rev Revision
> > Added: svn:eol-style
> > + native
> >
> > Modified: php/php-src/trunk/Zend/zend_compile.c
> > ===================================================================
> > --- php/php-src/trunk/Zend/zend_compile.c 2010-06-26 21:29:56 UTC
> (rev 300769)
> > +++ php/php-src/trunk/Zend/zend_compile.c 2010-06-26 22:05:13 UTC
> (rev 300770)
> > @@ -2909,13 +2909,20 @@
> > {
> > zend_uint i;
> >
> > - /* If it's a user function then arg_info == NULL means we don't
> have any parameters but we still need to do the arg number checks. We are
> only willing to ignore this for internal functions because extensions don't
> always define arg_info. */
> > + /* If it's a user function then arg_info == NULL means we don't
> have any parameters but
> > + * we still need to do the arg number checks. We are only willing
> to ignore this for internal
> > + * functions because extensions don't always define arg_info.
> > + */
> > if (!proto || (!proto->common.arg_info && proto->common.type !=
> ZEND_USER_FUNCTION)) {
> > return 1;
> > }
> >
> > - /* Checks for constructors only if they are declared in an
> interface */
> > - if ((fe->common.fn_flags & ZEND_ACC_CTOR) &&
> !(proto->common.scope->ce_flags & ZEND_ACC_INTERFACE)) {
> > + /* Checks for constructors only if they are declared in an
> interface,
> > + * or explicitly marked as abstract
> > + */
> > + if ((fe->common.fn_flags & ZEND_ACC_CTOR)
> > + && ((proto->common.scope->ce_flags & ZEND_ACC_INTERFACE) ==
> 0
> > + && (proto->common.fn_flags & ZEND_ACC_ABSTRACT) ==
> 0)) {
> > return 1;
> > }
> >
> >
> > --
> > PHP CVS Mailing List (http://www.php.net/)
> > To unsubscribe, visit: http://www.php.net/unsub.php
>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
--
Etienne Kneuss
http://www.colder.ch
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic