Everything

php

[RFC] Property Accessors v1.2 : isset / unset "failable"

php.internals

Clint M Priest

13 years ago

I'm opening up several new threads to get discussion going on the
remaining "being debated" categories referenced in this 1.1 -> 1.2
change spec:
https://wiki.php.net/rfc/propertygetsetsyntax-as-implemented/change-requests

------------------------------------------------------------------------
*isset / unset / attempted writes when no setter / attempted reads when
no getter**
*
Stas suggested that since there is presently no cases where these can
fail that with accessors these should never "fail."

Three possible ways to go (maybe others):

1.      If all cases can be tested for during compilation, prefer
compile failures.
2.      Let the compilation occur and at runtime when a disallowed
action is attempted, emit a warning and move on.
3.      As is currently, either at compilation or at runtime we issue a
fatal error and stop execution (probably least preferable if at runtime)

If "no failures" should be the way we want to go, then #2 or some
derivative makes the most sense.
------------------------------------------------------------------------

Thoughts?

--
-Clint

Stas Malyshev

13 years ago

Hi!

>  1.      If all cases can be tested for during compilation, prefer
>     compile failures.

Not likely. isset($foo->$bar) is completely opaque since we don't know
what $foo or $bar is.

>  2.      Let the compilation occur and at runtime when a disallowed
>     action is attempted, emit a warning and move on.
>  3.      As is currently, either at compilation or at runtime we issue a
>     fatal error and stop execution (probably least preferable if at runtime)

Actually, I think the right way is:

4. On isset(), if the value can be retrieved, return true. Otherwise,
return false (including the case when the value can not be retrieved
because of missing getter). Same holds for empty() but in reverse - if
isset() would return false, it'd return true and vice versa.
On unset($foo->bar), act exactly as if the code were $foo->bar = NULL;

Of course, this applies only for automatic definition of isset/unset.

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

Clint M Priest

13 years ago

That's basically what #2 is getting at, my only question is, emit a
warning or notice or not?

Technically returning false on an invalid isset() call could be
misleading without emitting some kind of notice or warning about it.

On 10/26/2012 9:56 AM, Stas Malyshev wrote:

> Hi!
>
>>   1.      If all cases can be tested for during compilation, prefer
>>      compile failures.
> Not likely. isset($foo->$bar) is completely opaque since we don't know
> what $foo or $bar is.
>
>>   2.      Let the compilation occur and at runtime when a disallowed
>>      action is attempted, emit a warning and move on.
>>   3.      As is currently, either at compilation or at runtime we issue a
>>      fatal error and stop execution (probably least preferable if at runtime)
> Actually, I think the right way is:
>
> 4. On isset(), if the value can be retrieved, return true. Otherwise,
> return false (including the case when the value can not be retrieved
> because of missing getter). Same holds for empty() but in reverse - if
> isset() would return false, it'd return true and vice versa.
> On unset($foo->bar), act exactly as if the code were $foo->bar = NULL;
>
> Of course, this applies only for automatic definition of isset/unset.

--
-Clint

Pierre Joye

13 years ago

hi Clint,

On Sat, Oct 27, 2012 at 7:39 PM, Clint Priest <cpriest@zerocue.com> wrote:

> That's basically what #2 is getting at, my only question is, emit a warning
> or notice or not?
>
> Technically returning false on an invalid isset() call could be misleading
> without emitting some kind of notice or warning about it.

isset is used for this exact purpose, avoid noisy scripts while
testing the existence (instance, init'ed, etc.) of a variable or
property. Raising a notice here sounds wrong, no matter in which
context.

Cheers,

--
Pierre

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

Clint M Priest

13 years ago

So... to be explicit here, you think in this situation:

class a {
public $b {
set($x) { $this->b = $x; }
}
}

$o = new a();

if(!isset($o->b)) {
/* delete files */
}
echo (int)isset($o->b);  /* This should return false and not emit any
sort of warning/notice? */

I mean specifically, there is no getter defined, therefore the result
if isset is indeterminate and while I can see it not causing execution
to stop I don't see it being a good idea to not warn the developer that
what they've attempted is not correct.  Without a getter, isset() is
not a legal call (since the value cannot be retrieved).

On Sunday, October 28, 2012 4:02:23 AM, Pierre Joye wrote:

> hi Clint,
>
> On Sat, Oct 27, 2012 at 7:39 PM, Clint Priest <cpriest@zerocue.com> wrote:
>> That's basically what #2 is getting at, my only question is, emit a warning
>> or notice or not?
>>
>> Technically returning false on an invalid isset() call could be misleading
>> without emitting some kind of notice or warning about it.
>
> isset is used for this exact purpose, avoid noisy scripts while
> testing the existence (instance, init'ed, etc.) of a variable or
> property. Raising a notice here sounds wrong, no matter in which
> context.
>
> Cheers,

--
-Clint

David Muir

13 years ago

On 29/10/12 03:02, Clint Priest wrote:

> So... to be explicit here, you think in this situation:
>
> class a {
>   public $b {
>      set($x) { $this->b = $x; }
>   }
> }
>
> $o = new a();
>
> if(!isset($o->b)) {
>   /* delete files */
> }
> echo (int)isset($o->b);  /* This should return false and not emit any
> sort of warning/notice? */
>
> I mean specifically, there is no getter defined, therefore the result
> if isset is indeterminate and while I can see it not causing execution
> to stop I don't see it being a good idea to not warn the developer
> that what they've attempted is not correct. Without a getter, isset()
> is not a legal call (since the value cannot be retrieved).
>

I agree with Pierre, isset() should never throw a warning/notice. The
primary use of isset (that I've seen in various code-bases) is to avoid
warnings/notices. Only rarely do I see it used in an expression to
control application logic. The above example should not give a warning,
and should return false. As far as an application is concerned, $o->b
doesn't exist because it can't be read.

David

Clint M Priest

13 years ago

That's pretty fair, that last statement... As far as an application is
concerned $o->b doesn't exist because it can't be read.

Seems as though some developers are going to want to know when they've
tried to violate it though... I dunno.  Personally I would consider it
error or warning worthy because if I made that mistake I would want to
know about it, rather than never been notified of it.

Is there another class of error that would make more sense?  Don't most
people turn off E_NOTICE errors?  Perhaps emit an E_STRICT?

On 10/28/2012 8:57 PM, David Muir wrote:

> On 29/10/12 03:02, Clint Priest wrote:
>> So... to be explicit here, you think in this situation:
>>
>> class a {
>>   public $b {
>>      set($x) { $this->b = $x; }
>>   }
>> }
>>
>> $o = new a();
>>
>> if(!isset($o->b)) {
>>   /* delete files */
>> }
>> echo (int)isset($o->b);  /* This should return false and not emit any
>> sort of warning/notice? */
>>
>> I mean specifically, there is no getter defined, therefore the result
>> if isset is indeterminate and while I can see it not causing
>> execution to stop I don't see it being a good idea to not warn the
>> developer that what they've attempted is not correct. Without a
>> getter, isset() is not a legal call (since the value cannot be
>> retrieved).
>>
>
> I agree with Pierre, isset() should never throw a warning/notice. The
> primary use of isset (that I've seen in various code-bases) is to
> avoid warnings/notices. Only rarely do I see it used in an expression
> to control application logic. The above example should not give a
> warning, and should return false. As far as an application is
> concerned, $o->b doesn't exist because it can't be read.
>
> David
>
>
>

--
-Clint

Stas Malyshev

13 years ago

Hi!

> Is there another class of error that would make more sense?  Don't most
> people turn off E_NOTICE errors?  Perhaps emit an E_STRICT?

I always run with E_NOTICE in development, that's kind of what E_NOTICE
is for :) I don't think isset() should produce any warnings/notices -
this is how it is now in PHP. Otherwise there's no point in having
isset() operation - you can check for NULL with read, whole point of
isset() is that you're checking both for read legality and read result,
and it's guaranteed not to be issuing warnings if this variable doesn't
exist (while reading may issue warnings).

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

Ford, Mike [LSS]

13 years ago

> -----Original Message-----
> From: Clint Priest [mailto:cpriest@zerocue.com]
> Sent: 28 October 2012 16:03
>
> So... to be explicit here, you think in this situation:
>
> class a {
>    public $b {
>       set($x) { $this->b = $x; }
>    }
> }
>
> $o = new a();
>
> if(!isset($o->b)) {
>    /* delete files */
> }
> echo (int)isset($o->b);  /* This should return false and not emit
> any
> sort of warning/notice? */
>
> I mean specifically, there is no getter defined, therefore the
> result
> if isset is indeterminate

I've been holding back from the accessors discussion as I'm hovering
around -0 on the whole thing, but this one is really very simple and
obvious as far as I'm concerned: no getter, means no value, means not
set, means isset() returns FALSE. Also, the description of isset()'s
return value (at http://php.net/isset) is "Returns TRUE if var exists
and has value other than NULL, FALSE otherwise"; this would also seem
to mandate returning FALSE (since a property with no getter effectively
would not exist in a "get" context).

Cheers!

Mike

--
Mike Ford,
Electronic Information Developer, Libraries and Learning Innovation,
Portland PD507, City Campus, Leeds Metropolitan University,
Portland Way, LEEDS,  LS1 3HE,  United Kingdom
E: m.ford@leedsmet.ac.uk     T: +44 113 812 4730

To view the terms under which this email is distributed, please go to http://disclaimer.leedsmet.ac.uk/email.htm

Stas Malyshev

13 years ago

Hi!

> So... to be explicit here, you think in this situation:
>
> class a {
>    public $b {
>       set($x) { $this->b = $x; }
>    }
> }
>
> $o = new a();
>
> if(!isset($o->b)) {
>    /* delete files */
> }
> echo (int)isset($o->b);  /* This should return false and not emit any
> sort of warning/notice? */

isset should return false, since $b is not set value. It should not
produce any warning. Of course (int) would produce 0 then ;)

> I mean specifically, there is no getter defined, therefore the result
> if isset is indeterminate and while I can see it not causing execution

No, the result is determinate - it's false. That's the point of isset()
in PHP and that's how it is used in existing code.

> to stop I don't see it being a good idea to not warn the developer that
> what they've attempted is not correct.  Without a getter, isset() is
> not a legal call (since the value cannot be retrieved).

isset() should always be legal. This is the way to check if $o->b is legal.

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

Clint M Priest

13 years ago

Would you say the same of unset?  Always benign?

On 10/29/2012 2:14 PM, Stas Malyshev wrote:

> Hi!
>
>> So... to be explicit here, you think in this situation:
>>
>> class a {
>>     public $b {
>>        set($x) { $this->b = $x; }
>>     }
>> }
>>
>> $o = new a();
>>
>> if(!isset($o->b)) {
>>     /* delete files */
>> }
>> echo (int)isset($o->b);  /* This should return false and not emit any
>> sort of warning/notice? */
> isset should return false, since $b is not set value. It should not
> produce any warning. Of course (int) would produce 0 then ;)
>
>> I mean specifically, there is no getter defined, therefore the result
>> if isset is indeterminate and while I can see it not causing execution
> No, the result is determinate - it's false. That's the point of isset()
> in PHP and that's how it is used in existing code.
>
>> to stop I don't see it being a good idea to not warn the developer that
>> what they've attempted is not correct.  Without a getter, isset() is
>> not a legal call (since the value cannot be retrieved).
> isset() should always be legal. This is the way to check if $o->b is legal.
>
>

--
-Clint

Clint M Priest

13 years ago

Bringing up this old issue a bit.

Nothing was ever said of unset?  Should unset be benign?

Since unset() is intended to take an action (rather than check on state)
shouldn't an invalid unset (one with a guarded property that doesn't
have a setter) emit a warning?

On 10/30/2012 10:37 PM, Clint Priest wrote:

> Would you say the same of unset?  Always benign?
>
>
> On 10/29/2012 2:14 PM, Stas Malyshev wrote:
>> Hi!
>>
>>> So... to be explicit here, you think in this situation:
>>>
>>> class a {
>>>     public $b {
>>>        set($x) { $this->b = $x; }
>>>     }
>>> }
>>>
>>> $o = new a();
>>>
>>> if(!isset($o->b)) {
>>>     /* delete files */
>>> }
>>> echo (int)isset($o->b);  /* This should return false and not emit any
>>> sort of warning/notice? */
>> isset should return false, since $b is not set value. It should not
>> produce any warning. Of course (int) would produce 0 then ;)
>>
>>> I mean specifically, there is no getter defined, therefore the result
>>> if isset is indeterminate and while I can see it not causing execution
>> No, the result is determinate - it's false. That's the point of isset()
>> in PHP and that's how it is used in existing code.
>>
>>> to stop I don't see it being a good idea to not warn the developer that
>>> what they've attempted is not correct.  Without a getter, isset() is
>>> not a legal call (since the value cannot be retrieved).
>> isset() should always be legal. This is the way to check if $o->b is
>> legal.
>>
>>
>

--
-Clint