Skip to content

Add compile-time warning on invalid parent use#3564

Closed
morrisonlevi wants to merge 1 commit intophp:masterfrom
morrisonlevi:parent
Closed

Add compile-time warning on invalid parent use#3564
morrisonlevi wants to merge 1 commit intophp:masterfrom
morrisonlevi:parent

Conversation

@morrisonlevi
Copy link
Copy Markdown
Contributor

This is a new attempt at fixing the issue from PR #3404.

zend_string *extends_name = zend_ast_get_str(extends_ast);
CG(active_class_entry_parent_name) = zend_resolve_class_name(
extends_name,
extends_ast->kind == ZEND_AST_ZVAL ? extends_ast->attr : ZEND_NAME_FQ);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer necessary, you can use ce->parent_name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - will rework the patch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic How do I know if it's safe to use parent or parent_name in that union?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During compilation it should always be parent_name. Otherwise the ZEND_ACC_LINKED flag.

@morrisonlevi
Copy link
Copy Markdown
Contributor Author

morrisonlevi commented Oct 8, 2018

Okay, the branch is now passing travis tests. Could I get another review?

@php-pulls
Copy link
Copy Markdown

Comment on behalf of petk at php.net:

Labelling

@nikic
Copy link
Copy Markdown
Member

nikic commented Jan 4, 2019

The changes for ::class handling here are not quite correct. If we add support for parent::class in constant expression contexts, then we also have to support it in cases where it can only be determined at runtime.

I've just fixed an existing related bug in 41af1e6.

@nikic
Copy link
Copy Markdown
Member

nikic commented Jan 4, 2019

I have refactored the ::class handling and introduced support for resolving parent::class during runtime constexpr eval.

@nikic
Copy link
Copy Markdown
Member

nikic commented Jan 4, 2019

I've committed the parent check as a hard error in a9e6667. I think the BC break here is both very minor and easy to fix, so going through a deprecation period is more trouble than it is worth.

@nikic nikic closed this Jan 4, 2019
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Jun 14, 2019
> - Core
>    Using "parent" inside a class without parent is deprecated, and will throw
>    a compile-time error in the future. Currently an error will only be
>    generated if/when the parent is accessed at run-time.

Refs:
* https://github.com/php/php-src/blob/42cc58ff7b2fee1c17a00dc77a4873552ffb577f/UPGRADING#L303
* php/php-src#3404
* php/php-src#3564
* php/php-src@a9e6667 (originally committed as hard compile time error)
* php/php-src@deb44d4 (reverted back to deprecation error)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants