-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix #44705, fix tuple_tfunc
on Union
containing Type{...}
#44725
Conversation
tuple_tfunc
on Union
containing Type{...}
test/compiler/inference.jl
Outdated
# https://github.com/JuliaLang/julia/issues/44705 | ||
import Core.Compiler: tuple_tfunc, widenconst | ||
@test widenconst(tuple_tfunc(Any[Type{Int}])) === Tuple{DataType} | ||
@test tuple_tfunc(Any[Union{Type{Int32},Type{Int64}}]) === Tuple{Type} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not Tuple{DataType}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need a more precise idea of widenconst that does widenconcrete, and we don't have that right now
|
0f0cfb1
to
8e61753
Compare
@@ -1554,6 +1558,8 @@ function tuple_tfunc(argtypes::Vector{Any}) | |||
else | |||
params[i] = Type | |||
end | |||
elseif !isvarargtype(x) && hasintersect(x, Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encountered a weird behavior here -- without this !isvarargtype(x)
check buildbot/xxx
builds fail with the following error:
Assertion failed: (!jl_is_vararg(x) && !jl_is_vararg(y)), function subtype, file /Users/julia/buildbot/worker/package_macos64/build/src/subtype.c, line 1261.
(e.g. https://build.julialang.org/#/builders/34/builds/4697)
while the same commit successfully builds w/o any problem on my machine (after make cleanall
or even a fresh clone).
Maybe buildbot uses a different method to build something, and in that case the bootstrap makes more use of interpreter or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turned out that I needed to enable "assert build".
This causes function assertx(xs::Vector{T}) where T
t = (xs[1], xs[2])
t::Tuple{T,T}
end
@code_typed optimize=false assertx(Type{<:Number}[Int,Int])
@code_typed optimize=true assertx(Type{<:Number}[Int,Int]) diff --git a/_master.jl b/_pr.jl
index 03768efc773..54ccceaa2cc 100644
--- a/_master.jl
+++ b/_pr.jl
@@ -2,8 +2,8 @@ julia> @code_typed optimize=false assertx(Type{<:Number}[Int,Int])
CodeInfo(
1 ─ %1 = Base.getindex(xs, 1)::Type{<:Number}
│ %2 = Base.getindex(xs, 2)::Type{<:Number}
-│ (t = Core.tuple(%1, %2))::Tuple{Type{<:Number}, Type{<:Number}}
-│ %4 = t::Tuple{Type{<:Number}, Type{<:Number}}
+│ (t = Core.tuple(%1, %2))::Tuple{Type, Type}
+│ %4 = t::Tuple{Type, Type}
│ %5 = Core.apply_type(Main.Tuple, $(Expr(:static_parameter, 1)), $(Expr(:static_parameter, 1)))::Core.Const(Tuple{Type{<:Number}, Type{<:Number}})
│ %6 = Core.typeassert(%4, %5)::Tuple{Type{<:Number}, Type{<:Number}}
└── return %6
@@ -13,6 +13,8 @@ julia> @code_typed optimize=true assertx(Type{<:Number}[Int,Int])
CodeInfo(
1 ─ %1 = Base.arrayref(true, xs, 1)::Type{<:Number}
│ %2 = Base.arrayref(true, xs, 2)::Type{<:Number}
-│ %3 = Core.tuple(%1, %2)::Tuple{Type{<:Number}, Type{<:Number}}
-└── return %3
+│ %3 = Core.tuple(%1, %2)::Tuple{Type, Type}
+│ Core.typeassert(%3, Tuple{Type{<:Number}, Type{<:Number}})::Tuple{Type{<:Number}, Type{<:Number}}
+│ %5 = π (%3, Tuple{Type{<:Number}, Type{<:Number}})
+└── return %5
) => Tuple{Type{<:Number}, Type{<:Number}} So it seems like this assertion error was wrongly erased before? I wonder if we want to disable constructing a container whose element type is |
We discussed this behavior of |
This commit adapts to the incoming upstream change of the Julia compiler. More specifically this change is necessary after JuliaLang/julia#44725, which fixes an internal bug of the compiler, which in turn makes the test suite of SquidGame fail without this change (if you're interested, please see <JuliaLang/julia#44725 (comment)>).
No description provided.