Skip to content

[RFC] Typed class constants - #5815

Closed
moliata wants to merge 8 commits into
php:masterfrom
moliata:typed_class_constants
Closed

[RFC] Typed class constants#5815
moliata wants to merge 8 commits into
php:masterfrom
moliata:typed_class_constants

Conversation

@moliata

@moliata moliata commented Jul 6, 2020

Copy link
Copy Markdown
Contributor

@kocsismate kocsismate added the RFC label Jul 6, 2020
$$->attr = $1; }
| method_modifiers T_CONST class_const_list ';'
{ $$ = zend_ast_create(ZEND_AST_CLASS_CONST_GROUP, $3, NULL);
| method_modifiers T_CONST type_expr_without_static class_const_list ';'

@moliata moliata Jul 6, 2020

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.

This needs to be optional_type_without_static instead of type_expr_without_static but since there are 4 shift/reduce conflicts using the former one, I am temporarily using type_expr_without_static.

...this is also the reason behind a bunch of failing tests.

Comment thread Zend/zend_vm_def.h
Comment thread Zend/zend_inheritance.c Outdated
Comment thread Zend/zend_inheritance.c Outdated
Comment thread Zend/tests/type_declarations/typed_class_constants_002.phpt

@kocsismate kocsismate left a comment

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.

I only have a few cosmetic questions/suggestions which shouldn't affect the RFC too much in so early a phase. :)

Comment thread ext/reflection/php_reflection.stub.php Outdated
Comment thread Zend/tests/type_declarations/typed_class_constants_002.phpt
Comment thread Zend/tests/type_declarations/typed_class_constants_004.phpt Outdated
Comment thread ext/reflection/php_reflection.stub.php
@iluuu1994

Copy link
Copy Markdown
Member

@moliata Are you planning on pursuing this RFC?

@moliata

moliata commented Jan 23, 2022

Copy link
Copy Markdown
Contributor Author

Hey, @iluuu1994

I do plan to continue pursuing this RFC, however, this implementation isn't really good and I will rewrite it completely from scratch when I get the time to do so. For now, I think it's fine that we close this PR but leave the RFC page still available on the wiki.

Best regards,
Ben

@iluuu1994 iluuu1994 closed this Jan 23, 2022
@iluuu1994

Copy link
Copy Markdown
Member

Thanks @moliata.

@moliata
moliata deleted the typed_class_constants branch January 23, 2022 13:04
@kocsismate

Copy link
Copy Markdown
Member

Hi @moliata,

Recently, I wanted to fix your problem with the parser. Even though I didn't reach far, @bwoebi helped me. Afterwards, I fixed a few smaller issues with your implementation... so your RFC now has an almost complete implementation: https://github.com/php/php-src/compare/master...kocsismate:php-src:const-type?expand=1

I strongly recommend though to split the abstract constant part of your proposal because of its possible controversiality and additional - unnecessary - complexity. I didn't even try to implement this.

@moliata

moliata commented Dec 17, 2022

Copy link
Copy Markdown
Contributor Author

@kocsismate For reference, I was confused about you mentioning the abstract part of my proposal. Apparently, Mark Niebergall modified my original proposal on the wiki, included abstract constant part and credited himself. I do not condone nor support that, so I plan to rewrite the original proposal.

@kocsismate

Copy link
Copy Markdown
Member

@moliata Did you have the chance to make some progress with the RFC?

@moliata

moliata commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

@kocsismate Despite having done some progress during the holidays, I have come to understanding that due to time constraints and many exam sessions that I am currently having, I won't be able to work on this properly any time soon. I would be glad if someone took upon this RFC and refined it.

@kocsismate

Copy link
Copy Markdown
Member

@moliata OK, I'd be happy to take the RFC over - of course by properly crediting your previous work. :)

The only additional thing I want is to make it possible to add types to global constants (const int FOO = 1). I need it for the stubs. If it's controversial then I'll split this to a separate vote. :)

@php php deleted a comment from smokefire69 Jan 25, 2023
@kocsismate

Copy link
Copy Markdown
Member

@moliata I've updated your original RFC and updated your implementation (#10444). The new version of the RFC doesn't contain anything surprising, it basically only makes typed constants work with objects - which are now supported by PHP.

@moliata

moliata commented Feb 5, 2023

Copy link
Copy Markdown
Contributor Author

@kocsismate Looks great to me! Thank you so much for your work and I wish you the best of luck during the voting phase.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants