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

Make rand(Integer) returns Integer #2007

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Make rand(Integer) returns Integer #2007

merged 2 commits into from
Sep 17, 2024

Conversation

soutaro
Copy link
Member

@soutaro soutaro commented Sep 15, 2024

Closes #1982

The rand implementation is a bit annoying for type checkers.

rand(0)    # Returns a Float
rand(1)    # Returns an Integer

The current type definition is sound that rand(1) returns Integer | Float, while it's really annoying because it returns Integer actually.

To make the type checkers more useful and match with our intuition, this PR makes the rand(1) returns Integer.

We assume the following pattern:

  • When we really want an Float result, we usually pass 0 literal to rand -- rand(0) should return an Float
  • When we pass some unknown integer value, we usually confirm the value is not 0, because we don't want Float result -- rand(i) can return Integer, no Float

The type definition uses literal type 0 to make type of rand(0) call Float. And other Integer cases returns an Integer.

This is unsound, but we believe this is more practical and useful.

Assuming we rarely pass an unknown integer value to `rand` for `Float` result.
@soutaro soutaro added this to the RBS 3.6 milestone Sep 15, 2024
@soutaro soutaro added this pull request to the merge queue Sep 17, 2024
Merged via the queue into master with commit 8c80c3d Sep 17, 2024
19 checks passed
@soutaro soutaro deleted the rand branch September 17, 2024 02:32
@soutaro soutaro added the Released PRs already included in the released version label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

1 participant