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

Potential type instability for integer promotions? #6255

Closed
wants to merge 1 commit into from

Conversation

mbauman
Copy link
Sponsor Member

@mbauman mbauman commented Mar 24, 2014

In changing the documentation for #6254, I realized that even the more complicated description of integer promotion is incorrect because promote(x) = (x,). While I haven't experienced this myself in real code, I think the current behavior here is surprising, particularly when calling promote with a splatted variable-length iterable. This PR adds specialized methods for promote with one SmallSigned/SmallUnsigned argument so the behavior matches the documentation.

With this PR, typeof(promote(uint8(3))) == (Uint,)

In changing the documentation for JuliaLang#6254, I realized that even the more complicated description of integer promotion is incorrect because `promote(x) = (x,)`.  While I haven't experienced this myself in real code, I think the current behavior here is surprising, particularly when calling promote with a splatted variable-length iterable. This PR adds specialized methods for promote with one SmallSigned/SmallUnsigned argument so the behavior matches the documentation.
@StefanKarpinski
Copy link
Sponsor Member

@JeffBezanson, thoughts?

@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 24, 2014

I realize this is pretty core-machinery level here, so perhaps it would have been better to simply open an issue. But I hacked this together and the tests pass on my 64-bit machine. (That said, a quick and basic grep doesn't show any splatted promote calls in all of Base/Test.)

@StefanKarpinski
Copy link
Sponsor Member

No, this is not an unreasonable thing to do. The core promotion machinery is super delicate in terms of overall performance and @JeffBezanson is the expert there, so we should wait for his eyeballs.

@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 24, 2014

You know, it just dawned on me that this isn't a good solution, either. There's no promotion when the types are the same, so we'd have the opposite problem with homogenous collections of small integers:

julia> typeof(promote(int8(1),int8(1)))
(Int8,Int8)
julia> typeof(promote(int8(1))) # with this patch
(Int64,)

@StefanKarpinski
Copy link
Sponsor Member

I'm warming to the idea that our basic arithmetic promotions need some reconsideration.

@johnmyleswhite
Copy link
Member

I'm warming to the idea that our basic arithmetic promotions need some reconsideration.

😈

@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 24, 2014

Also, I suppose that any time you're calling promote with a variable-length iterable, you're going to have some sort of type instability by definition (unless of course, it's a homogenous array, in which case the status quo is correct). So the title is a little ridiculous.

But I'd be in support of simplifying things here. It's telling that it's difficult to concisely describe the promotion behavior for integers (or keep in my head, for that matter!).

@mbauman
Copy link
Sponsor Member Author

mbauman commented Mar 25, 2014

Given that this is not going to be the direction to take here, I'll close this in favor of a more relevant issue. Perhaps re-open #1078?

@mbauman mbauman closed this Mar 25, 2014
@mbauman mbauman deleted the int-promotion branch April 7, 2014 21:34
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

3 participants