Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:101281
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.47 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAH0Uv3EU9pHu+NA5OPsycKWy2WQ41WK5vivTE8Z-oNP5_G=0cg@mail.gmail.com>
References: <CAH0Uv3EU9pHu+NA5OPsycKWy2WQ41WK5vivTE8Z-oNP5_G=0cg@mail.gmail.com>
Date: Sat, 9 Dec 2017 15:13:35 +0000
Message-ID: <CALtMV+Ow0GyVPsDV1yqS_HCUY=uh-FinwAQPEpNTg1xMx=D3gA@mail.gmail.com>
To: Andreas Hennings <andreas@dqxtech.net>, kalle@php.net
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a113c2f4ec74367055fe9beee"
Subject: Re: [PHP-DEV] instanceof survives non-object variables, but crashes
 on non-object constants.
From: aidantwoods@gmail.com (Aidan Woods)

--001a113c2f4ec74367055fe9beee
Content-Type: text/plain; charset="UTF-8"

I prefer the inconsistency between `$x instanceof \stdClass` and `1
instanceof \stdClass`, than adding a warning (or throwing) to `$x
instanceof \stdClass` when `$x = 1`.
I think `1 instanceof \stdClass === false` would be reasonable.


1. PHP has no means of locking a variable to a type (i.e. the variable
could be reassigned to a scalar at any point), so lack of an implicit
`is_object` check in `instanceof` would basically move to require an
explicit one in many cases it is used, if you want to be *sure* a warning
won't be thrown that is (baring in mind just using the variable earlier
might change its type non-visibly, since we don't have explicit caller
opt-in to pass-by-reference).

   This to say that IMO `$x instanceof stdClass` when `$x = 1` perhaps
shouldn't be considered broken usage because it is useful for it to perform
this check. Also consider that `is_object($x) === false` implies `$x` is
not an instance of any object (and in-particular not of the object you
might ask about with `instanceof`), so there is no reason `instanceof`
wouldn't be able to do this check.

   There is also a certain amount of irony about making a type-checking
operator start complaining when you give it the wrong type ;-)

2. There is already precedent for variables to act differently than
literals, and even constants in PHP when used in object operators, for
example
   ```
   $x = new \stdClass;
   const y = 'stdClass';
   $y = y;

   var_dump($x instanceof $x);
   var_dump($x instanceof $y);
   var_dump($x instanceof y);
   var_dump($x instanceof 'stdClass');
   var_dump($x instanceof new \stdClass);
   var_dump($x instanceof (new \stdClass));
   ```

   where the three lines return `false`, the third as (the const isn't
looked at, and no warning is thrown when checking against non-existent `y`
class). The last three lines throw errors.

   Point here is that, "should variables be treated like their literal
values?" is perhaps a bigger question, to which the answer at present seems
to be "no" of object operators.

   I'm not opposed to moving towards constancy, but I think there being
inconsistency isn't worth throwing in `$x instanceof \stdClass` when `$x =
1` (since inconsistency between variables and their values shouldn't be
unexpected when using object operators).

---
@Kalle

> We should just add a warning to the first example, it seems like an oversight
that it was left silent

&
>  That is fine for code that is broken in the first place. Similarly we
added a warning some years back about array to string conversions.

As a data-point, usage with non-objects is documented behaviour
http://php.net/manual/en/language.operators.type.php#example-115 so would
this be spec change to say this is incorrect, as opposed to being a bugfix?

Kind regards,
Aidan

On 9 December 2017 at 06:28, Andreas Hennings <andreas@dqxtech.net> wrote:

> The following (https://3v4l.org/A2Tp6) is ok, it simply returns false:
>
>     $x = 1;
>     $x instanceof \stdClass;
>
>
> The following (https://3v4l.org/IdSBu) gives a fatal error:
>
>     1 instanceof \stdclass;
>
> t think this behavior is inconsistent, and we should consider changing it.
>
> There are two options, but only one is BC.
>
> - Let 1 instanceof \stdClass return false, instead of crashing. -> seems BC
> - Let $x instanceof \stdClass crash, if $x is not an object. -> BC break.
>
> So it seems the first would the option we should take.
> This is also what hhvm does, according to https://3v4l.org/IdSBu.
>

--001a113c2f4ec74367055fe9beee--