-
Notifications
You must be signed in to change notification settings - Fork 87
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
Initial PHP 8 Support #67
Conversation
Thanks for the PR. I've left some feedback. |
@sorinsarca Is there any reason we don't include array, iterable, callable in the list of builtin types? I think certainly |
@GrahamCampbell all should be there, I don't know why they are missing |
// cc @driesvints for review |
hi, this will merge soon thanks, great Job. |
Sorry for the delay, this was so much harder than I thought it would be. I finally got all existing tests passing. I added some new test files during the development process because it was much easier for me to understand the array of I'm not sure what to do with these: https://github.com/opis/closure/pull/67/files#diff-4e781872947f7a06b71197212fc075bcR46 Also let me know about the new parser, instead of causing conflict I can try to reimplement. If the new parser is easier to work with, I might be able to use some of the knowledge I gathered while working on this PR to get it working there. In conclusion, my work here is done for now and this is ready for review |
|
I think we can merge this without attributes working. A priority is getting PHP 7 compatible syntax to work on PHP 8, which this PR already does. A follow-up PR can add support for every PHP 8.0 feature (which is still a slightly moving target until RC1 comes out, anyway). |
The lack of this is blocking Laravel from supporting PHP 8.0. |
@@ -910,6 +915,11 @@ protected function fetchItems() | |||
$name .= $token[1]; | |||
$alias = $token[1]; | |||
break; | |||
case T_NAME_QUALIFIED: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still unresolved.
Thanks! I'll try to look into Attributes now that we're in RC stage of PHP. |
I've opened a separate issue to track this: #80. It is possible that we might only want to bother with attribute support in v4 of this package (after this PR has been re-implemented for v4). We can discuss on that new issue. |
Ohh yeah thank you for merging, I'm working on PHP 8 support in PHP-DI and that was a blocker. Hey there @deleugpn, fun seeing you here! 😄 |
Should the README be updated to mention PHP 8.0 support? At present, only PHP 5 and 7 are mentioned. |
Provide PHP 8 Support.