Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[Override] attribute in trait does not check for parent class implementations #12189

Closed
jnoordsij opened this issue Sep 12, 2023 · 4 comments
Closed

Comments

@jnoordsij
Copy link

Description

When adding the #[Override] attribute to a trait method, then use-ing it in a class, a runtime error is triggered even if a parent class defines a matching method. This does not happen when an interface with matching signature is encountered.

The following excerpt from the rfc made me expect this would work:

Methods from a used trait behave as if the method definition was copied and pasted into the target class. Specifically the #[\Override] attribute on a trait method requires the existence of a matching method in a parent class or implemented interface.

The following code:

<?php

class A {
    public function foo(): void {}
}

interface I {
    public function foo(): void;
}

trait T {
    #[\Override]
    public function foo(): void {
        echo 'foo';
    }
}

// Works fine
class B implements I {
    use T;
}

// Works fine ("copied and pasted into the target class")
class C extends A {
    #[\Override]
    public function foo(): void {
        echo 'foo';
    }
}

// Does not work
class D extends A {
    use T;
}

Resulted in this output:

PHP Fatal error:  D::foo() has #[\Override] attribute, but no matching parent method exists in mwe.php on line 13
PHP Stack trace:
PHP   1. {main}() mwe.php:0

Fatal error: D::foo() has #[\Override] attribute, but no matching parent method exists in mwe.php on line 13

Call Stack:
    0.0002     364504   1. {main}() mwe.php:0

But I expected no output instead.

PHP Version

8.3.0RC1

Operating System

Windows 11 / Ubuntu 23.10 with WSL

@nielsdos
Copy link
Member

nielsdos commented Sep 12, 2023

Seems to be because of the flag removal code:

if (!check_only && child->common.scope == ce) {
	child->common.fn_flags &= ~ZEND_ACC_OVERRIDE;
}

The second condition doesn't trigger because the method doesn't belong to the class, it belongs to the trait. However, getting rid of the condition is a bad idea because it may load to modification inside shm as the trait is shared.
And duplicating the function seems like a very ugly solution.

@TimWolla
Copy link
Member

The second condition doesn't trigger because the method doesn't belong to the class, it belongs to the trait

Without looking yet into this again (I fear it's above my pay-grade based on your description), I wonder why it works fine for interfaces then 🤔

Looking at the tests, it appears this issue found the only case that is not covered by the tests 😔

TimWolla added a commit to TimWolla/php-src that referenced this issue Sep 15, 2023
…ng interface

Fixes phpGH-12189

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
TimWolla added a commit to TimWolla/php-src that referenced this issue Sep 15, 2023
@TimWolla TimWolla self-assigned this Sep 15, 2023
@jnoordsij
Copy link
Author

@TimWolla thanks for fixing!

@TimWolla
Copy link
Member

@jnoordsij Thank you for testing the release candidates and also for finding the bug 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants