Hi,

2010/6/28 Johannes Schl├╝ter <johan...@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

Reply via email to