Skip to content

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
Development

Successfully merging this pull request may close these issues.

Array returned from time_nanosleep() isn't seen to have the correct contents
4 participants