Fix for #53727 (is_subclass_of resolution with interfaces)

14 years ago by Ralph Schindler — view source

unread

Hi all,

Concerning: https://bugs.php.net/bug.php?id=53727

I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:

interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true

It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.

I've added a test file and all the current Zend/tests run as expected.

Thanks,
Ralph

14 years ago by david.zuelke@bitextender.com — view source

unread

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Typo there; that should be 'C', not 'B'.

David

14 years ago by Dmitry Stogov — view source

unread

Hi Ralph,

I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)

Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.

Thanks. Dmitry.

Hi all,

Concerning: https://bugs.php.net/bug.php?id=53727

I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:

interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true

It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.

I've added a test file and all the current Zend/tests run as expected.

Thanks,
Ralph

14 years ago by Ralph Schindler — view source

unread

Hey Dmitry,

is_a_impl() is an implementation that is shared by is_a() and
is_subclass_of() calls. There is a flag called only_subclass that
allows there to be switching behavior in is_a_impl().

The issue has been updated with a new patch that now takes objects into
account. It also has a comment in there that better explains what the
patch is doing.

I cannot assign it to you as I am not the original reporter, nor have
access, can you assign it to yourself?

Thanks,
Ralph

Hi Ralph,

I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)

Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.

Thanks. Dmitry.

Hi all,

Concerning: https://bugs.php.net/bug.php?id=53727

I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:

interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true

It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.

I've added a test file and all the current Zend/tests run as expected.

Thanks,
Ralph

14 years ago by Dmitry Stogov — view source

unread

Hi Ralph,

I reassigned it myself, and will take a look on Monday or Tuesday.

Thanks. Dmitry.

Hey Dmitry,

is_a_impl() is an implementation that is shared by is_a() and
is_subclass_of() calls. There is a flag called only_subclass that allows
there to be switching behavior in is_a_impl().

The issue has been updated with a new patch that now takes objects into
account. It also has a comment in there that better explains what the
patch is doing.

I cannot assign it to you as I am not the original reporter, nor have
access, can you assign it to yourself?

Thanks,
Ralph

Hi Ralph,

I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)

Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.

Thanks. Dmitry.

Hi all,

Concerning: https://bugs.php.net/bug.php?id=53727

I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:

interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true

It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.

I've added a test file and all the current Zend/tests run as expected.

Thanks,
Ralph

14 years ago by Dmitry Stogov — view source

unread

Hi Ralph,

Your patch was absolutely correct.
I've committed it with small improvements.

Thanks. Dmitry.

Hey Dmitry,

is_a_impl() is an implementation that is shared by is_a() and
is_subclass_of() calls. There is a flag called only_subclass that allows
there to be switching behavior in is_a_impl().

The issue has been updated with a new patch that now takes objects into
account. It also has a comment in there that better explains what the
patch is doing.

I cannot assign it to you as I am not the original reporter, nor have
access, can you assign it to yourself?

Thanks,
Ralph

Hi Ralph,

I agree with expected behavior, but didn't understand the patch and
which side effects it may have as well as the original implementation of
is_a_impl(). It looks for me like one huge bug :)

Please reassign the bug to me, if you like me to take a deeper look.
I can do it only on next week.

Thanks. Dmitry.

Hi all,

Concerning: https://bugs.php.net/bug.php?id=53727

I had put together a quick patch against PHP_5_3 to fix, for what I and
others might consider, an issue with is_subclass_of() returning false in
this situation:

interface A {}
class B implements A {}
var_dump(is_subclass_of('B', 'A')); // false, should be true

It returns false in situations where a class does not have a parent. If
a parent currently exists, the above works fine. For example:

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Attached is a patch against PHP_5_3, I can create a patch against
PHP_5_4 if need be.

I've added a test file and all the current Zend/tests run as expected.

Thanks,
Ralph

14 years ago by Paul Dragoonis — view source

unread

Correct.

I was hasty in that example, the first was copied & tested (and is reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

Regards,
Paul Dragoonis.

Thanks,
-ralph

interface A {}
class B implements A {}
class C extends B {}
var_dump(is_subclass_of('B', 'A')); // true

Typo there; that should be 'C', not 'B'.

David

14 years ago by david.zuelke@bitextender.com — view source

unread

Correct.

I was hasty in that example, the first was copied & tested (and is reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

instanceof is a language construct and only operates on object instances.

David

14 years ago by Paul Dragoonis — view source

unread

On Wed, Jun 29, 2011 at 9:22 PM, David Zülke
david.zuelke@bitextender.com wrote:

Correct.

I was hasty in that example, the first was copied & tested (and is reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

instanceof is a language construct and only operates on object instances.

I didn't spot you could pass a string into is_subclass_of() I thought
it was only objects. With that in mind it would be good to have
subclass_of() include interfaces.

David

14 years ago by Martin Scotta — view source

unread

On Wed, Jun 29, 2011 at 9:22 PM, David Zülke
david.zuelke@bitextender.com wrote:

On Wed, Jun 29, 2011 at 8:49 PM, Ralph Schindler ralph@smashlabs.com
wrote:

Correct.

I was hasty in that example, the first was copied & tested (and is
reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

instanceof is a language construct and only operates on object instances.

I didn't spot you could pass a string into is_subclass_of() I thought
it was only objects. With that in mind it would be good to have
subclass_of() include interfaces.

are you kidding, right?

$class = 'stdClass';
$instance = new $class;
var_dump( $instance instanceof $class );

David

14 years ago by david.zuelke@bitextender.com — view source

unread

are you kidding, right?

$class = 'stdClass';
$instance = new $class;
var_dump( $instance instanceof $class );

That still only works on instances. Not on class names. Which this dicussion is about.

David

14 years ago by Anthony Ferrara — view source

unread

are you kidding, right?

var_dump('Foo' instanceof 'Bar');

//Fatal Error: instanceof expects an object instance, constant given

On Wed, Jun 29, 2011 at 9:22 PM, David Zülke
david.zuelke@bitextender.com wrote:

On Wed, Jun 29, 2011 at 8:49 PM, Ralph Schindler ralph@smashlabs.com
wrote:

Correct.

I was hasty in that example, the first was copied & tested (and is
reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

instanceof is a language construct and only operates on object instances.

I didn't spot you could pass a string into is_subclass_of() I thought
it was only objects. With that in mind it would be good to have
subclass_of() include interfaces.

are you kidding, right?

$class = 'stdClass';
$instance = new $class;
var_dump( $instance instanceof $class );

David

14 years ago by Benjamin Eberlei — view source

unread

On Wed, 29 Jun 2011 22:22:54 +0200
David Zülke david.zuelke@bitextender.com wrote:

Correct.

I was hasty in that example, the first was copied & tested (and is reflected
in the test, as is that variation of what I wrote up.)

Either way, test and patch work in 5_3.

Doesn't this functionality confuse matters?

If this patch is added, is there now no difference between instanceof
and is_subclass_of(). If this is the case my question is then why do
we have two methods to do the same thing?

I thought instanceof was for parent classes + interfaces.. and
is_subclass_of() was just for parent classes.

instanceof is a language construct and only operates on object instances.

David

plus there is a difference if $a is a User is_subclass_of($a, 'User') returns false, $a instanceof User returns true.

14 years ago by Matt Pelmear — view source

unread

Seems odd to me that is_subclass_of returns true for interfaces... I never
realized that before.

Personally I would prefer a separate method to check for interfaces, since
they aren't really classes (at least in my head), but I suppose that would
break BC. (I've always wanted a method for checking if a class implements an
interface.)
So, maybe this patch makes sense to me, and some additional method for
checking only interfaces would be nice? (I can't think of a good name for
such a method.) Thoughts?

-Matt

14 years ago by David Muir — view source

unread

Seems odd to me that is_subclass_of returns true for interfaces... I never
realized that before.

Personally I would prefer a separate method to check for interfaces, since
they aren't really classes (at least in my head), but I suppose that would
break BC. (I've always wanted a method for checking if a class implements an
interface.)
So, maybe this patch makes sense to me, and some additional method for
checking only interfaces would be nice? (I can't think of a good name for
such a method.) Thoughts?

-Matt

As in class_implements() (shown in Ralph's example)?

David

14 years ago by Matt Pelmear — view source

unread

Not like class_implements(), since that requires processing the returned
array... Rather something that would function similarly to is_subclass_of()
(in that it returns a bool, not an array), but for interfaces.
I seem to run into a need for that on a fairly regular basis. (when I first
found class_implements() this is what I thought it did before reading the
docs)

-matt

Seems odd to me that is_subclass_of returns true for interfaces... I
never
realized that before.

Personally I would prefer a separate method to check for interfaces,
since
they aren't really classes (at least in my head), but I suppose that
would
break BC. (I've always wanted a method for checking if a class implements
an
interface.)
So, maybe this patch makes sense to me, and some additional method for
checking only interfaces would be nice? (I can't think of a good name for
such a method.) Thoughts?

-Matt

As in class_implements() (shown in Ralph's example)?

David

14 years ago by David Muir — view source

unread

Not like class_implements(), since that requires processing the returned
array... Rather something that would function similarly to is_subclass_of()
(in that it returns a bool, not an array), but for interfaces.
I seem to run into a need for that on a fairly regular basis. (when I first
found class_implements() this is what I thought it did before reading the
docs)

-matt

Oops. My bad. Missed the array_key_exists() part in the example.

David