[PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Ralph Schindler

Hey Internals,

In our work converting the Zend Framework library to namespaces, we came 
across some inconsistencies in constructor namings.


Clearly, with namespace support making class names shorter, we come back 
full circle where some class names collide with reserved words, thus we 
are forced to become creative with class names:


What was once:

  class Foo_Bar_Array {}

would become (1 to 1 mapping):

  namespace Foo\Bar; class Array {}

but must become

  namespace Foo\Bar; class ArrayBar {} (or ArrayObj, ArrayClass, etc)

We can work around these limitations.

With that in mind though, this is the bigger challenge we are faced with...


The following seems, to me, to be a BC break with regards to notices and 
E_STRICT usage:


  namespace Foo\Bar;
  class Filter {
public function __construct() { /* construct stuff */ }
public function filter($value) { /* return filtered */ }
  }

Produces:

  PHP Strict Standards:  Redefining already defined constructor for
  class Zend\Filter\Filter in [snip file] on line [snip line]


This worked in 5.2.x.
This worked in 5.3.0.
This stopped working in 5.3.1


Moreover, I do think that this is NOT the correct behavior.  While I 
know somewhere in the history of PHP we had an engine that preferred 
class names as constructor names, but thankfully- those days (one could 
only hope) are gone.


IMO, the engine should be backwards compatible as best as possbile in 
the 5.x series and should (with the respect of PHP4 tendencies), reward 
programmers using proper PHP5 constructs, like __construct.


(While I hate to use the english language as a crutch for the argument.. 
PHP is after all written in english, ... it is considered a best 
practice do have class names as nouns and methods as verbs.  Since there 
are a lot of cases where the same words are both the noun and verb.. the 
verb being the method being acted on in the class, the noun - combined 
with the shorter class names due to namespaces, I can see this becoming 
a bigger problem as more people embrace namespaces.)


Furthermore, static functions of the same name should not be marked as 
constructors by the engine in any situation (is there some BC we are 
keeping here? b/c I cannot see the use case.)





Attached is a patch that fixes this behavior (against PHP_5_3).  What it 
does:


  * If __construct is declared before a method of the same name as the 
class, you WILL NOT get a notice


  * If a method of the same name as the class appears before 
__construct(), you WILL get a notice, as the current behavior.


  (The idea here is that if you define __construct before anything 
else, as most people do, it will take priority and allow you to use the 
method name of the class name below that as normal)


  * Static methods of the same name as the class will NOT be marked as 
constructors (they currently are).  This might invalidate the check done 
inside zend_do_end_class_declaration() that checks to see if the 
constructor is static.  (Perhaps that first check becomes dead code?)




I am not sure where the current behavior came from. I looks like it got 
merged into 5_2_x but then backed out and merged into 5_3_x somewhere 
along the line after 5.3.0 was released.


In any case, considering we are not gonna get a context aware parser 
that will allow us to use reserved words for symbols like classes and 
functions, it would be nice to not have this seemingly artificial 
limitation on having method names that do not match the class name.


Thanks,
Ralph Schindler

References:

  http://bugs.php.net/bug.php?id=35634
  http://bugs.php.net/bug.php?id=48215
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 13b6c55..b7cd7b0 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -1285,13 +1285,11 @@ void zend_do_begin_function_declaration(znode 
*function_token, znode *function_n
zend_str_tolower_copy(short_class_lcname, 
short_class_name, short_class_name_length);
/* Improve after RC: cache the lowercase class name */
 
-   if ((short_class_name_length == name_len) && 
(!memcmp(short_class_lcname, lcname, name_len))) {
-   if (CG(active_class_entry)->constructor) {
-   zend_error(E_STRICT, "Redefining 
already defined constructor for class %s", CG(active_class_entry)->name);
-   } else {
+   if ((short_class_name_length == name_len) && 
(!memcmp(short_class_lcname, lcname, name_len)) && ((fn_flags & 
ZEND_ACC_STATIC) == 0)) {
+   if (!CG(active_class_entry)->constructor) {
CG(active_class_entry)->constructor = 
(zend_function *) CG(active_op_array);
}
-   } else if ((name_len == 
sizeof(ZEND_CONSTRUCTOR_FUNC_NAME)-1) && (!memcmp(lcname, 
ZEND_CONSTRUCTOR_FUNC_NAME, sizeof(ZEND_CONSTRUC

Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2011-08-16 Thread Ferenc Kovacs
On Sat, Apr 3, 2010 at 4:33 PM, Ralph Schindler  wrote:
> Stanislav Malyshev wrote:
>>
>> Hi!
>>
>>> class Filter {
>>> public function __construct() { /* construct stuff */ }
>>> public function filter($value) { /* return filtered */ }
>>> }
>>>
>>> Produces:
>>>
>>> PHP Strict Standards: Redefining already defined constructor for
>>> class Zend\Filter\Filter in [snip file] on line [snip line]
>>
>> I just checked - this code produces the same warning in 5.2 (without the
>> namespace of course), and in 5.3.0. So I don't see what changed in 5.3.1
>> exactly?
>
> Nothing changed, you are right, the behavior is there in all these versions.
>
> My PHP's for 5.3.0 and 5.3.1&2 have different default values for
> error_reporting, and since changing to E_STRICT at runtime is too late (file
> already compiled with error_reporting of php.ini), I was not triggering the
> code properly.
>
> (My 5.3.0 was shipped from apple, as was their php.ini which the default
> error_reporting value is E_ALL & ~E_NOTICE & ~E_DEPRECATED, not the newly
> recommended php.ini-development value of E_ALL | E_STRICT.  Also, in 5.2
> series, the PHP group never recommended to use E_STRICT in the php.ini,
> another reason the code was not triggered in my testing.)
>
> -ralph
>

sorry for resurrecting this thread:
I've had a discussion about this change recently, and I re-read the
whole thread to refresh my memories.
did I miss something, or did we really introduce this BC break based
on a "bogus" report?
I mean, based on Ralph's last email, it seems that the reported
warning was present in all previous versions, and the problem has
nothing to do with namespaces.

btw. I tried to reproduce the problem now, but the E_STRICT only
triggered, if I swapped the order of the __construct and filter
definition, which seems logical, cause if we have the __construct
first, then the classname ctor cannot override that(only the other way
around supported).
I looked around, and I found out that this change was introduced by
Felipe: https://bugs.php.net/bug.php?id=52160
he basically implemented Ralphs original patch/suggestion on his own.

does anybody else thinks that this is a good example what/how not to do things?

ps: Andi mentioned in this thread that we should mark the classname
ctors deprecated with the next version, if we want that, now is the
time.

-- 
Ferenc Kovács
@Tyr43l - http://tyrael.hu

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Stanislav Malyshev

Hi!


Clearly, with namespace support making class names shorter, we come back
full circle where some class names collide with reserved words, thus we
are forced to become creative with class names:


That might go away if we agreed to give up on case-insensitive class 
names (which btw don't really work that well with frameworks anyway - 
try autoloading class in wrong case from case-sensitive filesystem).
That would also make the engine simpler (and faster :) in a couple of 
places. But of course at cost of some BC pain.



The following seems, to me, to be a BC break with regards to notices and
E_STRICT usage:

namespace Foo\Bar;
class Filter {
public function __construct() { /* construct stuff */ }
public function filter($value) { /* return filtered */ }
}

Produces:

PHP Strict Standards: Redefining already defined constructor for
class Zend\Filter\Filter in [snip file] on line [snip line]


Hmm... Interesting issue. Without namespaces filter() obviously would be 
Filter's ctor, as class names aren't case sensitive now, remember? But 
with namespaces it gets tricky. Can we say filter() is still the ctor, 
and if not - we've got a problem of not being able to define a named 
ctor for a namespaced class. I'd say "good riddance" but some may disagree.



* If __construct is declared before a method of the same name as the
class, you WILL NOT get a notice


I don't like this. The behavior should not depend on other methods being 
defined. What if you refactored the class and moved the ctor out to the 
parent - and the you get a nasty surprise of filter() suddenly becoming 
new ctor?



(The idea here is that if you define __construct before anything else,
as most people do, it will take priority and allow you to use the method
name of the class name below that as normal)


In general, I actively do not like the idea of language behavior 
depending on the order of methods in the class. It's a bad idea. Code 
changes, and keeping track of this would be a nightmare.



* Static methods of the same name as the class will NOT be marked as
constructors (they currently are). This might invalidate the check done
inside zend_do_end_class_declaration() that checks to see if the
constructor is static. (Perhaps that first check becomes dead code?)


I would propose to have "class-named ctor" apply only to non-namespaced 
classes. Yes, that'd mean when you namespace a class you'd have to 
convert class-named ctor to __ctor, but if you want namespaces, you'd 
have to abandon your php 4 habits :)



In any case, considering we are not gonna get a context aware parser
that will allow us to use reserved words for symbols like classes and
functions, it would be nice to not have this seemingly artificial
limitation on having method names that do not match the class name.


Having parser that'd allow class named "array" is probably too hard, and 
there's a couple of cases (like function args) where it would plain just 
not work. Having class names Array is possible if we give up 
case-insensitivity (and I'd like to see that happen sometime - PHP is 
not a kid anymore :), but that'd probably require some consensus and RFC :)

--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Pierre Joye
hi,

On Thu, Apr 1, 2010 at 9:18 PM, Stanislav Malyshev  wrote:

> Hmm... Interesting issue. Without namespaces filter() obviously would be
> Filter's ctor, as class names aren't case sensitive now, remember? But with
> namespaces it gets tricky. Can we say filter() is still the ctor, and if not
> - we've got a problem of not being able to define a named ctor for a
> namespaced class. I'd say "good riddance" but some may disagree.

Well, I think the question here is more about dropping old style
constructor that case sensitive functions/methods name. I'm in favour
of dropping in php-next.

Cheers,
-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Stanislav Malyshev

Hi!


Well, I think the question here is more about dropping old style
constructor that case sensitive functions/methods name. I'm in favour
of dropping in php-next.


I don't feel comfortable with dropping class-named ctor altogether (big 
BC issue) but dropping it in NS-classes seems to be easier (technically, 
it's NOT the same name - the real class name is namespace\class) and 
would solve 99% of the problem without having almost any BC impact.

--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Ralph Schindler

Hey Stas,

The other option here is to simply go back to the behavior in 5.3.0 
where no notice is raised at all.  That is easy to accomplish (its just 
removing the notice inside the first if block of 
zend_do_begin_function_declaration().  It would also maintain 
consistency with 5.2.x.


[inline response]


* If __construct is declared before a method of the same name as the
class, you WILL NOT get a notice


I don't like this. The behavior should not depend on other methods being 
defined. What if you refactored the class and moved the ctor out to the 
parent - and the you get a nasty surprise of filter() suddenly becoming 
new ctor?


Consider this idea a band-aid.  It would basically allow for us to move 
forward in the 5.x branch with backwards compatibility until 6.x drops 
and PHP4 style constructors can go away forever.



* Static methods of the same name as the class will NOT be marked as
constructors (they currently are). This might invalidate the check done
inside zend_do_end_class_declaration() that checks to see if the
constructor is static. (Perhaps that first check becomes dead code?)


I would propose to have "class-named ctor" apply only to non-namespaced 
classes. Yes, that'd mean when you namespace a class you'd have to 
convert class-named ctor to __ctor, but if you want namespaces, you'd 
have to abandon your php 4 habits :)


Two notes here:

  1) I think that is a *fantastic idea* to drop PHP4 constructors from 
namespaced code. I'd be all for it!  I think that is fairly easy 
behavior to achieve.


  2) Another use case that is currently stifled by the current behavior:

 namespace Foo\Bar;
 class Filter {
 public static function filter() {}
 }

  In general, it seems like method names of the same as the class name 
should *NOT* be marked as the class constructor if they are static.


  The above produces this E_FATAL:

  PHP Fatal error:  Constructor Foo\Bar\Filter::filter() cannot be 
static in [snip]



In any case, considering we are not gonna get a context aware parser
that will allow us to use reserved words for symbols like classes and
functions, it would be nice to not have this seemingly artificial
limitation on having method names that do not match the class name.


Having parser that'd allow class named "array" is probably too hard, and 
there's a couple of cases (like function args) where it would plain just 
not work. Having class names Array is possible if we give up 
case-insensitivity (and I'd like to see that happen sometime - PHP is 
not a kid anymore :), but that'd probably require some consensus and RFC :)


Yeah, thats a bigger issue in an of itself that's better to be addressed 
in the far-off-but-hopefully-not-too-distant php6.



- Ralph Schindler

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Hannes Magnusson
On Thu, Apr 1, 2010 at 21:33, Stanislav Malyshev  wrote:
> Hi!
>
>> Well, I think the question here is more about dropping old style
>> constructor that case sensitive functions/methods name. I'm in favour
>> of dropping in php-next.
>
> I don't feel comfortable with dropping class-named ctor altogether (big BC
> issue) but dropping it in NS-classes seems to be easier (technically, it's
> NOT the same name - the real class name is namespace\class) and would solve
> 99% of the problem without having almost any BC impact.

Ouhgawd yes. Please drop support for oldstyle-ctor-in-namespaces. Its
a big PITA, and nearly no BC break. I would even consider it a bug fix
:]

-Hannes

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Stanislav Malyshev

Hi!


I don't like this. The behavior should not depend on other methods
being defined. What if you refactored the class and moved the ctor out
to the parent - and the you get a nasty surprise of filter() suddenly
becoming new ctor?


Consider this idea a band-aid. It would basically allow for us to move
forward in the 5.x branch with backwards compatibility until 6.x drops
and PHP4 style constructors can go away forever.


No, that's a pretty bad band-aid. I'd not be comfortable with such 
kludge being there, esp. when we are just establishing a new style for 
namespaced code.



In general, it seems like method names of the same as the class name
should *NOT* be marked as the class constructor if they are static.


Maybe, but that's seems to be too subtle distinction to have, and in the 
light of the previous issue (not having it be ctor ever) it doesn't 
matter anyway :)

--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-01 Thread Stanislav Malyshev

Hi!

The patch is simple: see attached. Doesn't break any tests except for 
ns_063 which specifically tests for this particular case. Any objections 
to having this in 5.3?

--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com
Index: zend_compile.c
===
--- zend_compile.c  (revision 297301)
+++ zend_compile.c  (working copy)
@@ -1270,22 +1270,13 @@
}
}
} else {
-   char *short_class_name;
-   int short_class_name_length;
-   char *short_class_lcname;
-
-   if ((short_class_name = 
zend_memrchr(CG(active_class_entry)->name, '\\', 
CG(active_class_entry)->name_length))) {
-   short_class_name_length = 
CG(active_class_entry)->name_length - (short_class_name - 
CG(active_class_entry)->name) - 1;
-   ++short_class_name;
-   } else {
-   short_class_name = CG(active_class_entry)->name;
-   short_class_name_length = 
CG(active_class_entry)->name_length;
-   }
-   short_class_lcname = do_alloca(short_class_name_length 
+ 1, use_heap);
-   zend_str_tolower_copy(short_class_lcname, 
short_class_name, short_class_name_length);
+   char *class_lcname;
+   
+   class_lcname = 
do_alloca(CG(active_class_entry)->name_length + 1, use_heap);
+   zend_str_tolower_copy(class_lcname, 
CG(active_class_entry)->name, CG(active_class_entry)->name_length);
/* Improve after RC: cache the lowercase class name */
 
-   if ((short_class_name_length == name_len) && 
(!memcmp(short_class_lcname, lcname, name_len))) {
+   if ((CG(active_class_entry)->name_length == name_len) 
&& (!memcmp(class_lcname, lcname, name_len))) {
if (CG(active_class_entry)->constructor) {
zend_error(E_STRICT, "Redefining 
already defined constructor for class %s", CG(active_class_entry)->name);
} else {
@@ -1338,7 +1329,7 @@
} else if (!(fn_flags & ZEND_ACC_STATIC)) {
CG(active_op_array)->fn_flags |= 
ZEND_ACC_ALLOW_STATIC;
}
-   free_alloca(short_class_lcname, use_heap);
+   free_alloca(class_lcname, use_heap);
}
 
efree(lcname);

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

RE: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-02 Thread Andi Gutmans
> -Original Message-
> From: Stanislav Malyshev [mailto:s...@zend.com]
> Sent: Thursday, April 01, 2010 12:34 PM
> To: Pierre Joye
> Cc: Ralph Schindler; internals
> Subject: Re: [PHP-DEV] On constructors: BC Break and Class compiler
> Improvements
> 
> Hi!
> 
> > Well, I think the question here is more about dropping old style
> > constructor that case sensitive functions/methods name. I'm in
favour
> > of dropping in php-next.
> 
> I don't feel comfortable with dropping class-named ctor altogether
(big BC
> issue) but dropping it in NS-classes seems to be easier (technically,
it's NOT
> the same name - the real class name is namespace\class) and would
solve
> 99% of the problem without having almost any BC impact.

I agree. I would drop class-named ctors within namespaced classes and
possibly in the next major version also do an E_STRICT for these in
regular classes to try and get people to convert to __construct().
At a time where there's an increased focus on exposing dynamic services
I think the class-named ctors is becoming increasingly problematic.

Andi

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-02 Thread Stanislav Malyshev

Hi!


class Filter {
public function __construct() { /* construct stuff */ }
public function filter($value) { /* return filtered */ }
}

Produces:

PHP Strict Standards: Redefining already defined constructor for
class Zend\Filter\Filter in [snip file] on line [snip line]


I just checked - this code produces the same warning in 5.2 (without the 
namespace of course), and in 5.3.0. So I don't see what changed in 5.3.1 
exactly?

--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-02 Thread Stanislav Malyshev

Hi!

So, I think we've got consensus about not having class-named ctors in 
namespaced classes in trunk, and unless I hear some screams I'll commit 
the patch early next week.


What about the 5.3? (BTW, I don't see any difference between 5.3.0 and 
anything later, could anybody point it to me?)

We could:
1. Kill the class-named ctors for NS in 5.3 (some BC break, though I 
have hard time believing anybody used it so far, but who knows)

2. Kill the warning and just ignore the other one if __ctor is there.
3. Leave everything as-is.

I kind of favor (1) but want to hear from people here and esp. RMs :)
--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-03 Thread Pierre Joye
hi,

On Sat, Apr 3, 2010 at 3:17 AM, Stanislav Malyshev  wrote:

> What about the 5.3? (BTW, I don't see any difference between 5.3.0 and
> anything later, could anybody point it to me?)
> We could:
> 1. Kill the class-named ctors for NS in 5.3 (some BC break, though I have
> hard time believing anybody used it so far, but who knows)
> 2. Kill the warning and just ignore the other one if __ctor is there.
> 3. Leave everything as-is.

I'm for 1) as a primary choice, or 3). 2) is just confusing.

Cheers,
-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-03 Thread Hannes Magnusson
On Sat, Apr 3, 2010 at 13:43, Pierre Joye  wrote:
> hi,
>
> On Sat, Apr 3, 2010 at 3:17 AM, Stanislav Malyshev  wrote:
>
>> What about the 5.3? (BTW, I don't see any difference between 5.3.0 and
>> anything later, could anybody point it to me?)
>> We could:
>> 1. Kill the class-named ctors for NS in 5.3 (some BC break, though I have
>> hard time believing anybody used it so far, but who knows)
>> 2. Kill the warning and just ignore the other one if __ctor is there.
>> 3. Leave everything as-is.
>
> I'm for 1) as a primary choice, or 3). 2) is just confusing.

Same here, I consider this a pure bugfix.

-Hannes

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-03 Thread Ralph Schindler

Stanislav Malyshev wrote:

Hi!


class Filter {
public function __construct() { /* construct stuff */ }
public function filter($value) { /* return filtered */ }
}

Produces:

PHP Strict Standards: Redefining already defined constructor for
class Zend\Filter\Filter in [snip file] on line [snip line]


I just checked - this code produces the same warning in 5.2 (without the 
namespace of course), and in 5.3.0. So I don't see what changed in 5.3.1 
exactly?


Nothing changed, you are right, the behavior is there in all these versions.

My PHP's for 5.3.0 and 5.3.1&2 have different default values for 
error_reporting, and since changing to E_STRICT at runtime is too late 
(file already compiled with error_reporting of php.ini), I was not 
triggering the code properly.


(My 5.3.0 was shipped from apple, as was their php.ini which the default 
error_reporting value is E_ALL & ~E_NOTICE & ~E_DEPRECATED, not the 
newly recommended php.ini-development value of E_ALL | E_STRICT.  Also, 
in 5.2 series, the PHP group never recommended to use E_STRICT in the 
php.ini, another reason the code was not triggered in my testing.)


-ralph

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-03 Thread Johannes Schlüter
On Thu, 2010-04-01 at 13:06 -0700, Stanislav Malyshev wrote:
> Hi!
> 
> The patch is simple: see attached. Doesn't break any tests except for 
> ns_063 which specifically tests for this particular case. Any objections 
> to having this in 5.3?

Given the feedback on the list I think it's ok.
Please make the BC break clear in the NEWS file so I remember to copy it
in the announcement.

Thanks,
johannes



-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-04 Thread Stanislav Malyshev

Hi!


Given the feedback on the list I think it's ok.
Please make the BC break clear in the NEWS file so I remember to copy it
in the announcement.


OK, done.
--
Stanislav Malyshev, Zend Software Architect
s...@zend.com   http://www.zend.com/
(408)253-8829   MSN: s...@zend.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-06 Thread Richard Quadling
2010/4/5 Stanislav Malyshev :
> Hi!
>
>> Given the feedback on the list I think it's ok.
>> Please make the BC break clear in the NEWS file so I remember to copy it
>> in the announcement.
>
> OK, done.
> --
> Stanislav Malyshev, Zend Software Architect
> s...@zend.com   http://www.zend.com/
> (408)253-8829   MSN: s...@zend.com
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

I've just done a quick check on PEAR/packages-all with regard to
classes using __construct vs the class name.

818 classes have __construct
1511 use the class name as the constructor
3563 files don't have a constructor

I excluded tests, examples and documentation (but some may have crept in).

Richard.
-- 
-
Richard Quadling
"Standing on the shoulders of some very clever giants!"
EE : http://www.experts-exchange.com/M_248814.html
EE4Free : http://www.experts-exchange.com/becomeAnExpert.jsp
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
ZOPA : http://uk.zopa.com/member/RQuadling

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-06 Thread Pierre Joye
2010/4/6 Richard Quadling :

> I've just done a quick check on PEAR/packages-all with regard to
> classes using __construct vs the class name.
>
> 818 classes have __construct
> 1511 use the class name as the constructor
> 3563 files don't have a constructor
>
> I excluded tests, examples and documentation (but some may have crept in).

You missed a critical part of this change:

When it is inside a Namespace.

I very much doubt that many PEAR packages actually use namespaces or
even 5.3 only (not badly meant).

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-06 Thread Hannes Magnusson
2010/4/6 Richard Quadling :
> 2010/4/5 Stanislav Malyshev :
>> Hi!
>>
>>> Given the feedback on the list I think it's ok.
>>> Please make the BC break clear in the NEWS file so I remember to copy it
>>> in the announcement.
>>
>> OK, done.
>> --
>> Stanislav Malyshev, Zend Software Architect
>> s...@zend.com   http://www.zend.com/
>> (408)253-8829   MSN: s...@zend.com
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>
>
> I've just done a quick check on PEAR/packages-all with regard to
> classes using __construct vs the class name.
>
> 818 classes have __construct
> 1511 use the class name as the constructor
> 3563 files don't have a constructor
>
> I excluded tests, examples and documentation (but some may have crept in).
>

None of those use namespaces, so there is no break there.

-Hannes

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-06 Thread Ferenc Kovacs
On Tue, Apr 6, 2010 at 1:32 PM, Hannes Magnusson  wrote:

> 2010/4/6 Richard Quadling :
> > 2010/4/5 Stanislav Malyshev :
> >> Hi!
> >>
> >>> Given the feedback on the list I think it's ok.
> >>> Please make the BC break clear in the NEWS file so I remember to copy
> it
> >>> in the announcement.
> >>
> >> OK, done.
> >> --
> >> Stanislav Malyshev, Zend Software Architect
> >> s...@zend.com   http://www.zend.com/
> >> (408)253-8829   MSN: s...@zend.com
> >>
> >> --
> >> PHP Internals - PHP Runtime Development Mailing List
> >> To unsubscribe, visit: http://www.php.net/unsub.php
> >>
> >>
> >
> > I've just done a quick check on PEAR/packages-all with regard to
> > classes using __construct vs the class name.
> >
> > 818 classes have __construct
> > 1511 use the class name as the constructor
> > 3563 files don't have a constructor
> >
> > I excluded tests, examples and documentation (but some may have crept
> in).
> >
>
> None of those use namespaces, so there is no break there.
>
> -Hannes
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
> should we check pear2?

Tyrael


Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2010-04-06 Thread Richard Quadling
On 6 April 2010 13:52, Ferenc Kovacs  wrote:
>
>
> On Tue, Apr 6, 2010 at 1:32 PM, Hannes Magnusson
>  wrote:
>>
>> 2010/4/6 Richard Quadling :
>> > 2010/4/5 Stanislav Malyshev :
>> >> Hi!
>> >>
>> >>> Given the feedback on the list I think it's ok.
>> >>> Please make the BC break clear in the NEWS file so I remember to copy
>> >>> it
>> >>> in the announcement.
>> >>
>> >> OK, done.
>> >> --
>> >> Stanislav Malyshev, Zend Software Architect
>> >> s...@zend.com   http://www.zend.com/
>> >> (408)253-8829   MSN: s...@zend.com
>> >>
>> >> --
>> >> PHP Internals - PHP Runtime Development Mailing List
>> >> To unsubscribe, visit: http://www.php.net/unsub.php
>> >>
>> >>
>> >
>> > I've just done a quick check on PEAR/packages-all with regard to
>> > classes using __construct vs the class name.
>> >
>> > 818 classes have __construct
>> > 1511 use the class name as the constructor
>> > 3563 files don't have a constructor
>> >
>> > I excluded tests, examples and documentation (but some may have crept
>> > in).
>> >
>>
>> None of those use namespaces, so there is no break there.
>>
>> -Hannes
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
> should we check pear2?
>
> Tyrael
>

Doh. Well at least you all know what you are talking about.

Same check on PEAR2...

It seems the only places where classnames are used as constructors are
in pyrus.phar and sandbox\SimpleChannelServer\pearscs.phar for

Pyrus\pyrus.phar : PEAR_Task_Replace_rw
Pyrus\pyrus.phar : PEAR_Task_Unixeol_rw
Pyrus\pyrus.phar : PEAR_Task_Windowseol_rw
Pyrus\pyrus.phar : Net_URL
sandbox\SimpleChannelServer\pearscs.phar : PEAR_Task_Replace_rw
sandbox\SimpleChannelServer\pearscs.phar : PEAR_Task_Unixeol_rw
sandbox\SimpleChannelServer\pearscs.phar : PEAR_Task_Windowseol_rw


As these are PEAR classes and don't aren't namespace, the current
PEAR2/all looks OK too.

Richard.
-- 
-
Richard Quadling
"Standing on the shoulders of some very clever giants!"
EE : http://www.experts-exchange.com/M_248814.html
EE4Free : http://www.experts-exchange.com/becomeAnExpert.jsp
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
ZOPA : http://uk.zopa.com/member/RQuadling

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] On constructors: BC Break and Class compiler Improvements

2018-03-30 Thread Rowan Collins

Hi all,

I thought people might be amused to hear that, almost exactly 8 years 
after it was proposed, this very reasonable decision caused a lot of 
head-scratching yesterday:



From:   Stanislav Malyshev 
Date:   2010-04-03 1:17:44

So, I think we've got consensus about not having class-named ctors in
namespaced classes in trunk, and unless I hear some screams I'll commit
the patch early next week.

What about the 5.3? (BTW, I don't see any difference between 5.3.0 and
anything later, could anybody point it to me?)
We could:
1. Kill the class-named ctors for NS in 5.3 (some BC break, though I
have hard time believing anybody used it so far, but who knows)
2. Kill the warning and just ignore the other one if __ctor is there.
3. Leave everything as-is.



While migrating an application off a horrifyingly old server, my friend 
(who shall remain anonymous to protect the innocent) could not 
understand why a private property of a class, initialised in the 
constructor, was null when the code ran on the new server. It turns out 
that the code in question looked like this:


namespace Foo\Bar;
class Baz {
    public function baz() {
    // initialisation here
    }
}

This code runs correctly *only* on PHP versions from 5.3.0 to 5.3.2 
inclusive, because in 5.3.3, Stas's patch mentioned above landed, and 
the method is no longer detected as a constructor. It just so happens 
that 5.3.2 was the version bundled with Ubuntu 10.04, and somehow the 
code remained untested on any other version in all this time.


Out of curiosity, I tracked down why this was changed, and found this 
thread. [https://marc.info/?l=php-internals&m=127014823111792] The 
awkwardness of allowing both constructor styles was only discovered as 
people started to move PEAR-style class names to namespaced ones, so 
tweaking it in 5.3.3 made perfect sense. But, as Stas said in the 
message quoted above, who knows what features people will take advantage 
of, documented or otherwise!


Regards,

--
Rowan Collins
[IMSoP]


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php