instanceof survives non-object variables, but crashes on non-object constants.

Adding a warning to something that used to be ok would break existing code.
I remember a number of cases where I used instanceof on possibly
non-objects.

if ($x instanceof C) {
return $x;
}
elseif ($x === NULL) {
...
}

All such code would then produce warnings.

Hi

We should just add a warning to the first example, it seems like an
oversight that it was left silent

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.

We should just add a warning to the first example, it seems like an
oversight that it was left silent

I strongly object! We ought to consider using instanceof to support
more type-checks than just classes. I think 1 instanceof int really
ought to return true for both literals and if they were variables:

$lhs = 1;
$rhs = 'int';
var_export($lhs instanceof $rhs);

The fact we do not have a single, unified way to check if some value
on the LHS is of the type on the RHS really bothers me.

where the three lines return false, the third [...]

Oops, that should say "the first two lines return true, the third
false" (the point here being that they return something, as opposed to
the later three, which throw).

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

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.

On Sun, Dec 10, 2017 at 12:09 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

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.

Neither of those "crashes", one of them returns fatal error, which is a
valid option for code that is obviously broken. It would also be a valid
option to return false, but since it is a valid outcome either way, I
don't think changing anything is necessary. It does not make any real
code easier to write (I can't see any legit reason to test numerics for
being instance of anything) and does not improve anything but abstract
"consistency", which in this case is not useful for anything.

The main issue seems to be that if people are not sure whether they have to
do an is_object() check before using instanceof, they just quickly test it
using "42 instanceof A" and then draw the entirely wrong conclusion from
the generated error. I've certainly seen this play out more than once.

Which is why I'd be in favor of changing instanceof to returning false in
this case.

Nikita

-----Original Message-----
From: php@golemon.com [mailto:php@golemon.com] On Behalf Of Sara
Golemon
Sent: Sunday, December 10, 2017 1:50 AM
To: Andreas Hennings andreas@dqxtech.net
Cc: PHP internals internals@lists.php.net
Subject: Re: [PHP-DEV] instanceof survives non-object variables, but crashes
on non-object constants.

On Sat, Dec 9, 2017 at 1:28 AM, 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 is one option, and it is BC.

• Let 1 instanceof \stdClass return false, instead of crashing. ->
  seems BC

This. There's nothing undefinable about "1 instanceof \stdClass", the clear
and obvious answer is: "No, it's not an instance of \stdClass, it's an integer."
The runtime result of false makes sense, the compile time error doesn't.
Let's relax the compile time error (fold it into a const false if you truly must)
and call it done. There's no need to be hostile to users just because the code
seems silly.

+1

Zeev

This. There's nothing undefinable about "1 instanceof \stdClass", the
clear and obvious answer is: "No, it's not an instance of \stdClass,
it's an integer." The runtime result of false makes sense, the
compile time error doesn't. Let's relax the compile time error (fold
it into a const false if you truly must) and call it done. There's no
need to be hostile to users just because the code seems silly.

This code doesn't just seem silly. For everything but testing whether it
works it is totally useless.

But we might turn the fatal error into a warning like we do it for "use" in
the global namespace: https://3v4l.org/ph7KW

Regards, Niklas

What would it hurt to allow instanceof to return types other than object?

$x instanceof NULL
$x instanceof string

and so on. This would be more powerful and useful than throwing an error
where one wasn't being thrown before. It also would let one operator detect
all the types, rather than the plethora of detect functions we currently
have.

On Sat, Dec 9, 2017 at 7:28 AM, 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.

I've prepared a PR for this change: https://github.com/php/php-
src/pull/2978

From the discussion I'm understanding that our consensus is to implement
this change, so if there are no further objection I'll merge this in a few
days.

Nikita