[RFC] Remove calls with incompatible Context

Will Fitch

14 years ago

I think this is a good idea.  I agree with the intention of throwing
E_DEPRECATED in 5.5, but what do you propose happen afterwards? Throw a
fatal error?  I would just like to make "removing (in the next version)" a
little more definitive.

On Mon, Jul 30, 2012 at 1:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt>wrote:

Gustavo Lopes

14 years ago

Em Mon, 30 Jul 2012 19:42:59 +0200, Will Fitch <willfitch@php.net>
escreveu:

> I think this is a good idea.  I agree with the intention of throwing
> E_DEPRECATED in 5.5, but what do you propose happen afterwards? Throw a
> fatal error?  I would just like to make "removing (in the next version)"
> a little more definitive.
>

As much as I'd like to have an error at call site here, the most sensible
option is to just have $this === null inside the callee, like when you do:

class A { function foo() {} }
A::foo(); // E_STRICT

I'll update the RFC with this.

--
Gustavo Lopes

Andrew Faulds

14 years ago

Slightly off-topic, $this being equal to NULL reminds me of how ECMAScript 5
changed the value of "this" to "undefined" in strict mode when not being called
as a method. I agree it's a more sensible solution, as the disappearance of a
variable might be more difficult to deal with than a variable changing value.
With the latter, you can just do var_dump($this) and see it is now NULL, which I
expect would be less confusing than it suddenly not being present.


On 30 July 2012 at 21:13 Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote:

> Em Mon, 30 Jul 2012 19:42:59 +0200, Will Fitch <willfitch@php.net>
> escreveu:
>
> > I think this is a good idea.  I agree with the intention of throwing
> > E_DEPRECATED in 5.5, but what do you propose happen afterwards? Throw a
> > fatal error?  I would just like to make "removing (in the next version)"
> > a little more definitive.
> >
>
> As much as I'd like to have an error at call site here, the most sensible
> option is to just have $this === null inside the callee, like when you do:
>
> class A { function foo() {} }
> A::foo(); // E_STRICT
>
> I'll update the RFC with this.
>
> --
> Gustavo Lopes
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

--
Andrew Faulds
http://ajf.me/

Anthony Ferrara

14 years ago

Gustavo,

On Mon, Jul 30, 2012 at 1:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt>wrote:

> https://wiki.php.net/rfc/**incompat_ctx<https://wiki.php.net/rfc/incompat_ctx>
>
> An RFC for deprecating and removing $this from incompatible context.
>
> Comments are welcome.

A few points.

1. Finally! It's caused a lot of WTF moments.

2. I have seen it in production code. Usually it wasn't intentional, but
changing it would definitely break things pretty bad... (Not a reason not
to change it, just pointing out that I have come across it).

3. You'd need to be careful to ensure that the error is raised only when
the called class is not in the same tree. http://codepad.viper-7.com/sOLCDu

Otherwise it would be difficult to call grand-parent code (skipping the
parent, if needed)...

Nice job.

Anthony

Todd Ruth

14 years ago

On Mon, 2012-07-30 at 19:31 +0200, Gustavo Lopes wrote:

> https://wiki.php.net/rfc/incompat_ctx
>
> An RFC for deprecating and removing $this from incompatible context.
>
> Comments are welcome.
>
> --
> Gustavo Lopes

I'm just a user, but I'd like to beg that the feature not be
removed, at least not without having an ini setting for at least
a couple of years.  As the RFC states, "This feature can, however,
be used  to implement trait-like behavior, and I'm sure someone
somewhere did such a thing."  I know in our company's code, such
uses were frequent.  I believe they are still common.  Refactoring
often doesn't fit the schedule.  I doubt we're unusual in allowing
classes to do more than they should instead of separating code
off into a trait or making other high level changes.  Note that
traits have only been available since March 1, 2012 (php 5.4.0).

I don't think axing register globals is very relevant.  Perhaps after
traits have been available for several years the comparison would
be better.  With register globals, a good alternative (the $_
variables) have been around for many years.  With traits, a good
alternative has been around for a few months.  I think the point
of mentioning register globals is just to say "We've made huge
BC breaks before."  That doesn't make it less painful.

Off-topic, but to show a potentially higher priority...
If you want to get rid of class related WTF, please put
get_class() back to the way it used to be.  Right now, calling
get_class() with a variable that is null is the same as calling
get_class with no arguments.  I can imagine why that might be
convenient for the implementation, but it is terribly confusing.

Thanks for listening.

- Todd

Will Fitch

14 years ago

On Mon, Jul 30, 2012 at 3:11 PM, Todd Ruth <truth@proposaltech.com> wrote:

> On Mon, 2012-07-30 at 19:31 +0200, Gustavo Lopes wrote:
> > https://wiki.php.net/rfc/incompat_ctx
> >
> > An RFC for deprecating and removing $this from incompatible context.
> >
> > Comments are welcome.
> >
> > --
> > Gustavo Lopes
>
> I'm just a user, but I'd like to beg that the feature not be
> removed, at least not without having an ini setting for at least
> a couple of years.  As the RFC states, "This feature can, however,
> be used  to implement trait-like behavior, and I'm sure someone
> somewhere did such a thing."  I know in our company's code, such
> uses were frequent.  I believe they are still common.  Refactoring
> often doesn't fit the schedule.  I doubt we're unusual in allowing
> classes to do more than they should instead of separating code
> off into a trait or making other high level changes.  Note that
> traits have only been available since March 1, 2012 (php 5.4.0).

> I don't think axing register globals is very relevant.  Perhaps after
> traits have been available for several years the comparison would
> be better.  With register globals, a good alternative (the $_
> variables) have been around for many years.  With traits, a good
> alternative has been around for a few months.  I think the point
> of mentioning register globals is just to say "We've made huge
> BC breaks before."  That doesn't make it less painful.

Software evolution at this level will always have some aches and pains for
someone.  This is a feature that has been discouraged (E_STRICT) for a very
long time, and, as pointed out in the RFC, is around only to be compatible
with PHP 4.  This should've been nixed with the death of PHP 4 IMO.  If we
waited around for everyone to update their codebase before
deprecating/removing features, it would never happen.  The point of
deprecation is to give a final warning that a feature is going to be
removed in a future release.  This should be your company's indication to
make the necessary changes.

That said, the length of deprecation is up in the air.  I'm not sure that
removing in the release immediately after deprecation is the right route,
but deprecating this in the next release *should* happen.

Gustavo Lopes

14 years ago

Em Mon, 30 Jul 2012 21:11:58 +0200, Todd Ruth <truth@proposaltech.com>
escreveu:

> On Mon, 2012-07-30 at 19:31 +0200, Gustavo Lopes wrote:
>> https://wiki.php.net/rfc/incompat_ctx
>>
>> An RFC for deprecating and removing $this from incompatible context.
>>
>> Comments are welcome.
>>
>
> I'm just a user, but I'd like to beg that the feature not be
> removed, at least not without having an ini setting for at least
> a couple of years.  As the RFC states, "This feature can, however,
> be used  to implement trait-like behavior, and I'm sure someone
> somewhere did such a thing."  I know in our company's code, such
> uses were frequent.  I believe they are still common.  Refactoring
> often doesn't fit the schedule.  I doubt we're unusual in allowing
> classes to do more than they should instead of separating code
> off into a trait or making other high level changes.  Note that
> traits have only been available since March 1, 2012 (php 5.4.0).

I don't doubt that you're not 'unusual in allowing classes to do more than
they should instead of separating code off into a trait or making other
high level changes'. I DO doubt, however, that this particular form of
class bloating is usual or "not unusual". However, it will be impossible
to collect reliable data on this.

> I don't think axing register globals is very relevant.  Perhaps after
> traits have been available for several years the comparison would
> be better.  With register globals, a good alternative (the $_
> variables) have been around for many years.  With traits, a good
> alternative has been around for a few months.  I think the point
> of mentioning register globals is just to say "We've made huge
> BC breaks before."  That doesn't make it less painful.

1. I don't think how long traits have been around particularly relevant
here. The relevant question, IMO, is whether an alternative exists. And if
you're upgrading to a version where call-with-incompatible-context does
not exist, traits will definitely be an option. At most, it's a point for
the minority of code that must run in multiple versions, but

2. I don't even agree with the premise. By the time this is removed,
traits will have been around for quite some time. Plus,

3. There are other low-cost alternatives, namely the obvious one: pass the
object via an extra parameter instead of operating on $this directly and
unconditionally. This is really easy to do.

4. Maybe register_globals was not the best example, as you have very easy
workarounds. But consider the removal of call-time pass by reference
(deprecated in 5.3 -- if I'm not mistaken -- and removed in 5.4). It's a
change that does require significant refactoring (not to mention the
awkwardness of having to use call_user_func_array() to pass by reference
to __call methods), and it had certainly a much wider use than this.

--
Gustavo Lopes

Christopher Jones

14 years ago

On 07/30/2012 01:32 PM, Gustavo Lopes wrote:

> 3. There are other low-cost alternatives, namely the obvious one: pass the object via an extra parameter instead of operating on $this directly and unconditionally. This is really easy to do.

This kind of thing should be mentioned in the RFC.

Chris

--
christopher.jones@oracle.com
http://twitter.com/#!/ghrd

Gustavo Lopes

14 years ago

Em 2012-07-31 1:55, Christopher Jones escreveu:

> On 07/30/2012 01:32 PM, Gustavo Lopes wrote:
>
>> 3. There are other low-cost alternatives, namely the obvious one:
>> pass the object via an extra parameter instead of operating on $this
>> directly and unconditionally. This is really easy to do.
>
> This kind of thing should be mentioned in the RFC.
>

I've now added this information to the RFC:

https://wiki.php.net/rfc/incompat_ctx#alternatives

--
Gustavo Lopes

Stas Malyshev

14 years ago

Hi!

> https://wiki.php.net/rfc/incompat_ctx
>
> An RFC for deprecating and removing $this from incompatible context.

IMO makes sense. Nulling-out $this makes most of the sane code (like
de-facto static functions not marked as static) work, and breaking
insane code in 5.5 in acceptable.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Ferenc Kovacs

14 years ago

On Mon, Jul 30, 2012 at 7:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt>wrote:

> https://wiki.php.net/rfc/**incompat_ctx<https://wiki.php.net/rfc/incompat_ctx>
>
> An RFC for deprecating and removing $this from incompatible context.
>
> Comments are welcome.
>
> --
> Gustavo Lopes
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

for the record I got bitten by that once or twice in the past and first
time it was a huuuge wtf.
definitively a +1 from me.

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

Etienne Kneuss

14 years ago

On Mon, Jul 30, 2012 at 7:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote:

> https://wiki.php.net/rfc/incompat_ctx
>
> An RFC for deprecating and removing $this from incompatible context.
>
> Comments are welcome.

4+ years ago Marcus already wanted to clean that up in PHP6.

Definitely +1.

Best,

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

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

Andrew Faulds

14 years ago

On 03/08/12 23:36, Etienne Kneuss wrote:

> On Mon, Jul 30, 2012 at 7:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote:
>> https://wiki.php.net/rfc/incompat_ctx
>>
>> An RFC for deprecating and removing $this from incompatible context.
>>
>> Comments are welcome.
> 4+ years ago Marcus already wanted to clean that up in PHP6.
>
> Definitely +1.
>
> Best,
>> --
>> Gustavo Lopes
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>
>

While we're at it,

$a = 'this';
$$a = new StdClass();

should be an error.
Also $GLOBALS['this'] = new StdClass();

https://bugs.php.net/bug.php?id=52428

--
Andrew Faulds
http://ajf.me/

reeze

14 years ago

在 2012年8月4日星期六，上午6:40，Andrew Faulds 写道：

> On 03/08/12 23:36, Etienne Kneuss wrote:
> > On Mon, Jul 30, 2012 at 7:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt (mailto:glopes@nebm.ist.utl.pt)> wrote:
> > > https://wiki.php.net/rfc/incompat_ctx
> > >
> > > An RFC for deprecating and removing $this from incompatible context.
> > >
> > > Comments are welcome.
> > 4+ years ago Marcus already wanted to clean that up in PHP6.
> >
> > Definitely +1.
> >
> > Best,
> > > --
> > > Gustavo Lopes
> > >
> > > --
> > > PHP Internals - PHP Runtime Development Mailing List
> > > To unsubscribe, visit: http://www.php.net/unsub.php
> > >
> >
> >
>
> While we're at it,
>
> $a = 'this';
> $$a = new StdClass();
>
> should be an error.
> Also $GLOBALS['this'] = new StdClass();
>
> https://bugs.php.net/bug.php?id=52428

then so does: extract(array('this' => 'scalar')); and the similar situation that may import variables.

Julien Pauli

14 years ago

On Sat, Aug 4, 2012 at 12:40 AM, Andrew Faulds <ajf@ajf.me> wrote:

> While we're at it,
>
> $a = 'this';
> $$a = new StdClass();
>
> should be an error.
> Also $GLOBALS['this'] = new StdClass();
>
> https://bugs.php.net/bug.php?id=52428

Hehe, I already caught all those executor tricks, like :
<?php
${'th'.'is'} = 'what I want';

In fact, "$this" is checked at compile time. The compiler checks for
the letters t+h+i+s to be found in the code, (usually throught
strcmp())  and acts as we know.
$this is never checked at execution, that's why we can use such tricks
to do illegal things.
I discussed that with [I cant remember who] (Felipe wasn't it?) few
times ago, and he told me that checking for $this to be used at
execution time would be too time consuming (I assume it would need a
lookup at every variable read operation, so that's why).
Plus, if someone wants to shoot himself in the foot, then let him do it.

Coming back to the RFC subject :
- Excuse me to perhaps ask for too much but: why don't we simply
forbid any static method call to non static method ?

I mean, that's something I've never understood why PHP allows an
instance method to be called statically ? That's a non sense to my
eyes, I've never used such a thing in my PHP  life. That's ugly,
illogic and simply wrong.
If you want to call a method statically, then make it static and then
prevent it from carrying any $this information.

But I see that most of you would prefer keeping this behavior and
somehow fix it by just turning $this to NULL in incompatible context.
I would have thrown an E_FATAL-what're-u-doing error :-p

Julien

Julien Pauli

14 years ago

On Sat, Aug 4, 2012 at 12:36 AM, Etienne Kneuss <colder@php.net> wrote:

> On Mon, Jul 30, 2012 at 7:31 PM, Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote:
>> https://wiki.php.net/rfc/incompat_ctx
>>
>> An RFC for deprecating and removing $this from incompatible context.
>>
>> Comments are welcome.

After several explanations with Etienne on static calls , then I see
why my purpose is not very right :)
So I +1 Gustavo's proposal which seem pretty in the right way things
must be done IMO :)

Thank you

Julien