Everything

php

[patch] Late static bindings (LSB)

php.internals

Etienne Kneuss

19 years ago

Hi internals,

here is a patch that implements Late static bindinds in a way that
minimizes the performance hits that were feared.
There is no significant slowdown or memory usage increase when running
Zend/bench.php, which I assume is a
good enough bench for that kind of matter, as it involves a stupid
amount of (recursive) function calls.

You can also find the patch here:
http://patches.colder.ch/Zend/late_static_bindings_take6.patch?markup

Here is a document that describes its usage:
http://colder.ch/news/08-24-2007/28/late-static-bindings-expl.html

Regards,

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Dmitry Stogov

19 years ago

Hi Etienne,

We already have patch for late static binding that is very similar to yours.
If you have time, please compare them.
From quick look I see that our patch more accurate (it supports constants
and runtime function calls)
Does our patch miss something that your patch does?

Thanks. Dmitry.

Etienne Kneuss

18 years ago

Hello,

sorry for the late reply, I am on vacations these weeks and only have a
sparse access to web.
I was aware that my patch missed supports for callbacks, and let it that
way on purpose as I
planned to do some cleanup work on callbacks.

It looks like you placed the work necessary to have LSB in a different
place than mine,
generating more occurences and hence a quite bigger patch size. I'd like
to see what implications it has.

Sadly, the patch you gave me is not easily patchable with the current
HEAD, do you have an up to date version ?

Thanks in advance

Dmitry Stogov wrote:

> Hi Etienne,
>
> We already have patch for late static binding that is very similar to yours.
> If you have time, please compare them.
> >From quick look I see that our patch more accurate (it supports constants
> and runtime function calls)
> Does our patch miss something that your patch does?
>
> Thanks. Dmitry.
>
>
>> -----Original Message-----
>> From: Etienne Kneuss [mailto:colder@php.net]
>> Sent: Friday, August 24, 2007 5:19 PM
>> To: internals@lists.php.net
>> Subject: [PHP-DEV] [patch] Late static bindings (LSB)
>>
>>
>> Hi internals,
>>
>> here is a patch that implements Late static bindinds in a way that
>> minimizes the performance hits that were feared.
>> There is no significant slowdown or memory usage increase
>> when running
>> Zend/bench.php, which I assume is a
>> good enough bench for that kind of matter, as it involves a stupid
>> amount of (recursive) function calls.
>>
>> You can also find the patch here:
>> http://patches.colder.ch/Zend/late_static_bindings_take6.patch?markup
>>
>> Here is a document that describes its usage:
>> http://colder.ch/news/08-24-2007/28/late-static-bindings-expl.html
>>
>> Regards,
>>
>> --
>> Etienne Kneuss
>> http://www.colder.ch
>>
>> Men never do evil so completely and cheerfully as
>> when they do it from a religious conviction.
>> -- Pascal
>>
>>
>>

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Dmitry Stogov

18 years ago

Attached.

Thanks. Dmitry.

Etienne Kneuss

18 years ago

Hi Dmitry,

your patch fails with one test of mine. It may show a difference in the
conception we have of LSB's usage, consider the following snip:

------%<-------------------------------------
<?php

class A {
public static function foo() {
static::who();
}

public static function who() {
echo __CLASS__."\n";
}
}

class B extends A {
public static function test() {
A::foo();
}

public static function who() {
echo __CLASS__."\n";
}
}
B::test();

------%<-------------------------------------

Since the call A::foo() is completely defined and that no "fall back"
occurs, I guess "A" is more expected as a result of this script.
Your patch will return B. I discussed this matter quite heavily on
#php.pecl and the expectations were also that "A" should get returned here.

Now if returning "A" is what we really want for PHP's LSB, I'm not sure
whether it's easily doable with your patch. To bring my patch up
to date with the latest functionalities like constants access through
constant()/defined() and callbacks wouldn't require much work.

But if my patch, which uses a slightly different approach (which seems
to be simpler according to patch size), have no way to go in anyway, I'd
like to know it before
I start wasting time on it again.

Thanks in advance.

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Mike Lively

18 years ago

> Since the call A::foo() is completely defined and that no "fall back"
> occurs, I guess "A" is more expected as a result of this script.
> Your patch will return B. I discussed this matter quite heavily on
> #php.pecl and the expectations were also that "A" should get returned
> here.

I've taken a look at this patch and for the most part I agree that your code
SHOULD return 'A'. However, one thing that I think is very important is that
you are able to somehow pass-thru who the caller is when dealing with
inheritance.

For instance:

<?php

class Foo
{
const TEST_CONST = 'foo';

public function test()
{
return static::TEST_CONST;
}
}

class Bar extends Foo
{
const TEST_CONST = 'bar';
}

echo Foo::test()."\n";
echo Bar::test()."\n";

?>

This works as expected outputting:
foo
bar

However, this concept becomes very inflexible when dealing with some aspects
of inheritance. Consider a child class that needs to do additional work in
its test() method and then called the parent test() method to do the normal
work.

<?php

class Bar2 extends Foo
{
const TEST_CONST = 'bar2';

public function test()
{
//do class specific things
//try to continue the function chain
return parent::test();
}
}

?>

This will also output 'foo'. While I agree that in this case this concept
does make sense, I do think it is important for there to be a way to have
this function return 'bar2'. The way that would most make sense to me and
still somewhat follow the rules is to change the return to static::test();
However this causes a seg fault in your current patch. I will do a little
more checking to see why you are segfaulting here.

In either case without having some way to chain callers this is going to
become a very annoying problem when utilizing inheritance with late static
binding.

--

Mike Lively
http://www.ds-o.com

Etienne Kneuss

18 years ago

Michael Lively wrote:

>> Since the call A::foo() is completely defined and that no "fall back"
>> occurs, I guess "A" is more expected as a result of this script.
>> Your patch will return B. I discussed this matter quite heavily on
>> #php.pecl and the expectations were also that "A" should get returned
>> here.
>
> I've taken a look at this patch and for the most part I agree that
> your code SHOULD return 'A'. However, one thing that I think is very
> important is that you are able to somehow pass-thru who the caller is
> when dealing with inheritance.
>
> For instance:
>
> <?php
>
> class Foo
> {
>        const TEST_CONST = 'foo';
>
>        public function test()
>        {
>                return static::TEST_CONST;
>        }
> }
>
> class Bar extends Foo
> {
>        const TEST_CONST = 'bar';
> }
>
> echo Foo::test()."\n";
> echo Bar::test()."\n";
>
> ?>
>
> This works as expected outputting:
> foo
> bar
>
> However, this concept becomes very inflexible when dealing with some
> aspects of inheritance. Consider a child class that needs to do
> additional work in its test() method and then called the parent test()
> method to do the normal work.
>
> <?php
>
> class Bar2 extends Foo
> {
>        const TEST_CONST = 'bar2';
>
>        public function test()
>        {
>                //do class specific things
>                //try to continue the function chain
>                return parent::test();
>        }
> }
>
> ?>
>
> This will also output 'foo'. While I agree that in this case this
> concept does make sense, I do think it is important for there to be a
> way to have this function return 'bar2'. The way that would most make
> sense to me and still somewhat follow the rules is to change the
> return to static::test(); However this causes a seg fault in your
> current patch. I will do a little more checking to see why you are
> segfaulting here.
>

That's an endless recursion, hence the segfault ;)

> In either case without having some way to chain callers this is going
> to become a very annoying problem when utilizing inheritance with late
> static binding.

I aggree that it could be a problem indeed, but you usually use LSB to
avoid having to redeclare static functions in child classes, if you do
anyway, LSB looses its interest.

Regards

>
> --
>
> Mike Lively
> http://www.ds-o.com

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Mike Lively

18 years ago

----- Original Message -----
From: "Etienne Kneuss" <colder@php.net>
To: <internals@lists.php.net>
Sent: Sunday, September 16, 2007 10:50 AM
Subject: Re: [PHP-DEV] [patch] Late static bindings (LSB)

>> However this causes a seg fault in your current patch. I will do a little
>> more checking to see why you are segfaulting here.
>>
> That's an endless recursion, hence the segfault ;)

Yah, quickly found that out after running gdb

>
>> In either case without having some way to chain callers this is going to
>> become a very annoying problem when utilizing inheritance with late
>> static binding.
> I aggree that it could be a problem indeed, but you usually use LSB to
> avoid having to redeclare static functions in child classes, if you do
> anyway, LSB looses its interest.

You of course wouldn't HAVE to redeclare the static function. Occasionally
you just need to do a little extra in a child class and if this breaks lsb
(or makes it innefective) with no work around (static::) then that would be
no good.

Stanislav Malyshev

18 years ago

> Since the call A::foo() is completely defined and that no "fall back"
> occurs, I guess "A" is more expected as a result of this script.
> Your patch will return B. I discussed this matter quite heavily on
> #php.pecl and the expectations were also that "A" should get returned here.

I think you are right, static:: is supposed to mean the context of the
call, so if A::foo() is called then static means A.

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

Etienne Kneuss

18 years ago

Hello,

I made the update anyway as it looked like it wouldn't take much time.

Attached is a updated version of my patch, along with all the tests
related to it:
http://patches.colder.ch/Zend/late_static_bindings_take7.patch?markup

--
Etienne Kneuss
http://www.colder.ch

Men never do evil so completely and cheerfully as
when they do it from a religious conviction.
-- Pascal

Dmitry Stogov

18 years ago

Hi Etienne,

At first thank you for catching the issue.

I deside not to delay review of your patch and found only one serious bug.
It doesn't handle nested calls (test lsb_017.phpt in my patch).

--TEST--
ZE2 nested calls
--FILE--
<?php
class A {
public static function test($x=null) {
if (!is_null($x)) {
echo "$x\n";
}
return get_caller_class();
}
}

class B extends A {
}
class C extends A {
}
class D extends A {
}

echo A::test(B::test(C::test(D::test())))."\n";
?>
==DONE==
--EXPECT--
D
C
B
A
==DONE==

Your patch outputs 4 D.

BTW I like other aspects of your patch.
I think your get_called_class() is better than my get_caller_class(). (I
changed this in my new patch).

I also made the same behavior that you maintained in private email. I didn't
removed the corresponding code yet but wrapped it with #ifdef ZEND_LSB2.
(tests lsb_018.phpt and test_019.phpt must be failed without it).

I also made several optimizations.

The latest version of my patch is attached.
I think if you fix the bug that I maintained before our patches will near
identical.
BTW may be your patch will be better, so please make a mixture of our
patches.
I assume they have exactly the same behavior, so we need the fastest one.

Thanks. Dmitry.

Mike Lively

18 years ago

There is a serious problem with both of these patches as they are now.

I understand the principal behind
<?

class A {
static public function test() { echo get_called_class() }
}

class B extend A {
static public function test2() { A::test(); }
}

B::test2()

?>

Returning 'A'. But I don't think that it is wise making this change without
providing a way to still allow LSB to work further down an inheritance
structure.

The whole purpose behind late static binding is to offer greater flexibility
when dealing with inheritance of static functions. I understand that the
aspect of the original patch returning 'B' for the code above is incorrect.
However, consider the following scenario.

<?php

class A {
static public function test() {
echo get_called_class()."\n";
}
}

class B {
static public function test() {
// Performing additional needed tasks.
parent::test();
}
}

?>

In class B it is impossible to perform additional work in ::test() and allow
class A to still do it's work while still being able to identify the class
that originated the set of calls. I don't think it can be denied that this
is incredibly inflexible and is somewhat against the reason for introducing
late static binding.

I think it is important that a solution is found to this problem, whether it
is allowing parent:: to forward on the 'called class' or introducing a new
keyword:: that will forward on the 'called class'.

Also, just as a somewhat obvious side note I think the ability to do what I
have mentioned will be expected by the OO programmers (which is who lsb is
for):
<?php

class A
{
public function test()
{
echo get_class($this);
}
}

class B extends A
{
public function test()
{
// Performing additional needed tasks.
parent::test();
}
}

$b = new B();
$b->test();
?>

Returns 'B' with no problem


----- Original Message -----
From: "Dmitry Stogov" <dmitry@zend.com>
To: "'Etienne Kneuss'" <colder@php.net>
Cc: <internals@lists.php.net>; "Stanislav Malyshev" <stas@zend.com>; "Andi
Gutmans" <andi@zend.com>
Sent: Tuesday, September 18, 2007 5:59 AM
Subject: RE: [PHP-DEV] [patch] Late static bindings (LSB)


Hi Etienne,

At first thank you for catching the issue.

I deside not to delay review of your patch and found only one serious bug.
It doesn't handle nested calls (test lsb_017.phpt in my patch).

--TEST--
ZE2 nested calls
--FILE--
<?php
class A {
public static function test($x=null) {
if (!is_null($x)) {
echo "$x\n";
}
return get_caller_class();
}
}

class B extends A {
}
class C extends A {
}
class D extends A {
}

echo A::test(B::test(C::test(D::test())))."\n";
?>
==DONE==
--EXPECT--
D
C
B
A
==DONE==

Your patch outputs 4 D.

BTW I like other aspects of your patch.
I think your get_called_class() is better than my get_caller_class(). (I
changed this in my new patch).

I also made the same behavior that you maintained in private email. I didn't
removed the corresponding code yet but wrapped it with #ifdef ZEND_LSB2.
(tests lsb_018.phpt and test_019.phpt must be failed without it).

I also made several optimizations.

The latest version of my patch is attached.
I think if you fix the bug that I maintained before our patches will near
identical.
BTW may be your patch will be better, so please make a mixture of our
patches.
I assume they have exactly the same behavior, so we need the fastest one.

Thanks. Dmitry.

> -----Original Message-----
> From: Etienne Kneuss [mailto:colder@php.net]
> Sent: Sunday, September 16, 2007 8:18 PM
> To: Dmitry Stogov
> Cc: internals@lists.php.net
> Subject: Re: [PHP-DEV] [patch] Late static bindings (LSB)
>
>
> Hello,
>
> I made the update anyway as it looked like it wouldn't take much time.
>
> Attached is a updated version of my patch, along with all the tests
> related to it:
> http://patches.colder.ch/Zend/late_static_bindings_take7.patch?markup
>
> --
> Etienne Kneuss
> http://www.colder.ch
>
> Men never do evil so completely and cheerfully as
> when they do it from a religious conviction.
> -- Pascal
>
>

--------------------------------------------------------------------------------

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

--------------------------------------------------------------------------------


No virus found in this incoming message.
Checked by AVG Free Edition.
Version: 7.5.487 / Virus Database: 269.13.21/1010 - Release Date: 9/15/2007
7:54 PM

Dmitry Stogov

18 years ago

Hi Michael,

Good catch.
You can get the behavior that you expect by enabling (#ifdef ZEND_LSB2) in
my patch.
Tests lsb_018.phpt and lsb_019.phpt are especially for this behavior.

I am not sure which behavior shouldbe in final patch.
It seems like support for inheritance provides more flixebility, but makes
concept harder to understand.

Thanks. Dmitry.

Lukas Kahwe Smith

18 years ago

Dmitry Stogov wrote:

> I am not sure which behavior shouldbe in final patch.
> It seems like support for inheritance provides more flixebility, but makes
> concept harder to understand.

Well inheritance is an advanced OO concept. As such its something that
requires a bit of getting into. But making it inconsistent will not help
ease of use.

regards,
Lukas

Marcus Börger

18 years ago

Hello Lukas,

right, we already have inheritance all over so we should do it here as
well. Nice work so far.

marcus

Wednesday, September 19, 2007, 9:07:16 AM, you wrote:

> Dmitry Stogov wrote:

>> I am not sure which behavior shouldbe in final patch.
>> It seems like support for inheritance provides more flixebility, but makes
>> concept harder to understand.

> Well inheritance is an advanced OO concept. As such its something that
> requires a bit of getting into. But making it inconsistent will not help
> ease of use.

> regards,
> Lukas

Best regards,
Marcus