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

Array.new(size, &block) accepts UInt32, other Ints #3248

Merged
merged 1 commit into from
Sep 4, 2016

Conversation

will
Copy link
Contributor

@will will commented Sep 4, 2016

Not a huge deal, but something I just ran into. Either this, or changing the signature to be explicit that it actually requires only Int32.

In the common case, I assume the #to_i would be a noop.

@asterite
Copy link
Member

asterite commented Sep 4, 2016

Good point. Maybe it's better if we restrict it to Int32, so that if someone wants to pass an Int64 it doesn't work and requires an explicit conversion.

I guess the problem you ran into is that the yielded value had the type of the argument passed to new, right?

@asterite
Copy link
Member

asterite commented Sep 4, 2016

Hm, I'm seeing that everywhere else we indeed cast from any Int to Int32, so for now we can do that. Later we can review this and take a better choice.

@asterite asterite merged commit fad767c into crystal-lang:master Sep 4, 2016
@will
Copy link
Contributor Author

will commented Sep 4, 2016

I guess the problem you ran into is that the yielded value had the type of the argument passed to new, right?

The yield is where the error happened, but the cause was passing a uint I got off the network into new without casting it first. I figured since they were both positive integers it would work.

In this patch, I put the cast before the yield so it would just happen once, instead of every yield.

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.

None yet

2 participants