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

Unusal handling of Integer typed empty arrays #17811

Closed
gabgoh opened this issue Aug 4, 2016 · 9 comments
Closed

Unusal handling of Integer typed empty arrays #17811

gabgoh opened this issue Aug 4, 2016 · 9 comments
Assignees

Comments

@gabgoh
Copy link

gabgoh commented Aug 4, 2016

In Julia 0.4.5, I could perform the following operations with no trouble.

julia> I = Integer[]
0-element Array{Integer,1}

julia> I = abs(I)
0-element Array{Any,1}

julia> push!(I,1)
1-element Array{Any,1}:
 1

An identical sequence in Julia 0.5 gives a stackoverflow error.

julia> I = Integer[]
0-element Array{Integer,1}

julia> I = abs(I)
0-element Array{Union{},1}

julia> push!(I,1)
ERROR: StackOverflowError:
 in convert(::Type{Union{}}, ::Int64) at ./sysimg.jl:55 (repeats 80000 times)

i realize this is a little esoteric, but what's going on? Everything's fine with Int64's

julia> I = Int64[]
0-element Array{Int64,1}

julia> abs(I)
0-element Array{Int64,1}
@gabgoh gabgoh changed the title Unusal handling of typed, empty arrays Unusal handling of Integer typed empty arrays Aug 4, 2016
@KristofferC
Copy link
Sponsor Member

Ref: #17717

@gabgoh
Copy link
Author

gabgoh commented Aug 4, 2016

nailed the issue, thanks!

i'm curious what Union{} is though, does it simply mean "no possible type" rendering the array unusable?

@KristofferC
Copy link
Sponsor Member

Just to point out, this works if you have it in a function. It is a global variable / type inference issue;

julia> function f()
           I = Integer[]
           I = abs(I)
           push!(I,1)
       end
f (generic function with 1 method)

julia> f()
1-element Array{Any,1}:
 1

@stevengj
Copy link
Member

stevengj commented Aug 4, 2016

@KristofferC, just tried it in 0.5 master and it still doesn't work in a function. Note also that you get the same Union{} type for map(abs, Integer[]) and broadcast(abs, Integer[]).

@JeffBezanson and @pabloferz, I thought from #17172 and #17389 that we were using type inference for the empty-array case. Why is inference here returning Union{} and not Integer or Any?

@KristofferC
Copy link
Sponsor Member

You are right @stevengj , I must have tried in the wrong window.

@pabloferz
Copy link
Contributor

pabloferz commented Aug 4, 2016

@stevengj Type inference is being used when the array type is concrete and/or the function is type stable for the input types. For the empty non-inferable case it returns Union{}. I don't know the rationale behind this specific case. Here Any seems to be a better option to me.

@stevengj
Copy link
Member

stevengj commented Aug 4, 2016

@pabloferz and @JeffBezanson, I thought the conclusion of our many many discussions of this topic was:

  • type inference would be used for the empty-array case (regardless of whether the inferred type is concrete!)
  • type inference would be used for the non-empty case if the inferred type is concrete (this is purely an optimization)
  • for the non-empty case with non-concrete inferred type, the actual values would be used.

@JeffBezanson
Copy link
Sponsor Member

@stevengj I'll try to update to this behavior and see what happens.

However, the problem with mutation is more general. Unless we return an Any array, it's always possible you'll try to push something later that doesn't match the element type. But I agree Union{} is a pathologically bad case of this where you can't push anything into the array.

@stevengj
Copy link
Member

stevengj commented Aug 4, 2016

Won't type inference always produce a type that is at least as broad as the actual types that would be produced in the non-empty case? (Any if inference fails to figure anything out at all?) So the mutation issue would be no worse than in the non-empty case.

@JeffBezanson JeffBezanson self-assigned this Aug 5, 2016
JeffBezanson pushed a commit that referenced this issue Aug 9, 2016
This tests that the result type of an empty comprehension is big enough to hold the possible results.
JeffBezanson pushed a commit that referenced this issue Aug 9, 2016
This tests that the result type of an empty comprehension is big enough to hold the possible results.
tkelman pushed a commit that referenced this issue Aug 11, 2016
tkelman pushed a commit that referenced this issue Aug 11, 2016
This tests that the result type of an empty comprehension is big enough to hold the possible results.
(cherry picked from commit 93c6061)
mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
)

This tests that the result type of an empty comprehension is big enough to hold the possible results.
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

No branches or pull requests

5 participants