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

fix time_nanosleep return type signature #1787

Merged
merged 1 commit into from Oct 3, 2022

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 3, 2022

@staabm staabm changed the base branch from 1.9.x to 1.8.x October 3, 2022 15:53
@@ -12308,7 +12308,7 @@
'tidyNode::isPhp' => ['bool'],
'tidyNode::isText' => ['bool'],
'time' => ['positive-int'],
'time_nanosleep' => ['array{0:0|positive-int,1:0|positive-int}|bool', 'seconds'=>'int', 'nanoseconds'=>'int'],
'time_nanosleep' => ['array{seconds:0|positive-int,nanoseconds:0|positive-int}|bool', 'seconds'=>'int', 'nanoseconds'=>'int'],
Copy link
Contributor

Choose a reason for hiding this comment

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

would an int-range int<0, max> be better than unioning a constant int and positive-int?

Copy link
Contributor Author

@staabm staabm Oct 3, 2022

Choose a reason for hiding this comment

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

I don't think any of the two is "better" - they are equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

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

isn't a single IntegerRangeType preferable to a UnionType containing two different types with regards to performance?
semantically they are they same i agree, but will phpstan simplify it to reduce the php object count from 3 to 1 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

0|positive-int will be normalized to int<0, max> so in that sense they're equivalent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you find a example in which we find a measurable difference we should have a closer look.
as we don't have one, I would assume its a micro difference

Copy link
Contributor

Choose a reason for hiding this comment

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

@rvanvelzen thank you, i'm still learning lots about phpstan's type system so forgive any silly questions i may have!

@ondrejmirtes ondrejmirtes merged commit 8462fbd into phpstan:1.8.x Oct 3, 2022
@ondrejmirtes
Copy link
Member

Thank you.

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