Conversation
|
@nanosoldier |
|
@nanosoldier |
| @test_throws ErrorException("Tuple field type cannot be Union{}") Tuple{Int, Vararg{Union{},1}} | ||
| @test_throws ErrorException("Tuple field type cannot be Union{}") Tuple{Vararg{Union{},1}} | ||
| @test Tuple{} <: Tuple{Vararg{Union{},N}} where N | ||
| @test !(Tuple{} >: Tuple{Vararg{Union{},N}} where N) |
There was a problem hiding this comment.
Why is that? Is there an N for which the LHS is not >: the RHS? Isn't N==0 the only admissible choice?
There was a problem hiding this comment.
yeah, subtyping cannot be allowed to fully know that though, since we don't really want the concrete type Tuple{} to be equal to a non-concrete type
There was a problem hiding this comment.
Perhaps we'd better normalize Tuple{A,B,C,Vararg{Union{},N}} where N to Tuple{A,B,C} after this change?
There was a problem hiding this comment.
No, that makes a different type, where N is free
There was a problem hiding this comment.
What about Tuple{} >: Tuple{Vararg{Union{}}}? There's a check for <:, but should these be ==?
There was a problem hiding this comment.
Tuple{Vararg{Union{}}} is invalid (it is just Tuple{} now)
There was a problem hiding this comment.
I guess that raises the possibility of making Vararg{Union{}} itself also directly throw an error. I was trying to be conservative in allowing Union{} but constructing a different sort of object. There could be a case to be made that Union{} should not appear their either though (which would allow deleting several more tests as non-applicable also)
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
|
PkgEval crashed. @nanosoldier |
|
@nanosoldier |
|
Your package evaluation job has completed - possible new issues were detected. |
|
From MCMCChains: And indeed, on master: julia> Base.Broadcast.eltypes((Union{}[],))
Tuple{Union{}}Should that be reworked to return |
|
Perhaps we'd better rewake #39295 and drop |
|
Ah, right, the result of |
|
Right, I just forgot to fix up a couple calls. Before: after @nanosoldier |
|
@nanosoldier |
|
@nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
|
Your package evaluation job has completed - possible new issues were detected. |
Type intersection assumed it was equal to Union{}, so this makes it
unconstructable so that holds true. This is similar to what the
NamedTuple constructor does.
Secondarily, this fixes an inference bug where it would create
Vararg{Union{}} and then incorrectly handle that fieldtype.
- Fixes #32392
- Addresses part of the concerns discussed in
#24614 (comment)
- Addresses part of the issues presented in
#26175
- May allow improving jl_type_equality_is_identity
(https://github.com/JuliaLang/julia/pull/49017/files#diff-882927c6e612596e22406ae0d06adcee88a9ec05e8b61ad81b48942e2cb266e9R986)
- May allow improving intersection (finish_unionall can be more
aggressive at computing varval for any typevars that appears in
covariant position and has lb=Union{} and ub=leaf type)
|
@vtjnash Similar error as above in testset REPL reported on buildkite and on my local build (macOS ARM): details |
…ion{}} otherwise
Refs: #49111
Type intersection assumed it was equal to Union{}, so this makes it
unconstructable so that holds true. This is similar to what the
NamedTuple constructor does.
Secondarily, this fixes an inference bug where it would create
Vararg{Union{}} and then incorrectly handle that fieldtype.
- Fixes JuliaLang#32392
- Addresses part of the concerns discussed in
JuliaLang#24614 (comment)
- Addresses part of the issues presented in
JuliaLang#26175
- May allow improving jl_type_equality_is_identity
(https://github.com/JuliaLang/julia/pull/49017/files#diff-882927c6e612596e22406ae0d06adcee88a9ec05e8b61ad81b48942e2cb266e9R986)
- May allow improving intersection (finish_unionall can be more
aggressive at computing varval for any typevars that appears in
covariant position and has lb=Union{} and ub=leaf type)
this was changed in JuliaLang/julia#49111 for older julia versions this cast should be a no-op
this was changed in JuliaLang/julia#49111 for older julia versions this cast should be a no-op
Type intersection assumed it was equal to Union{}, so this makes it unconstructable so that holds true. This is similar to what the NamedTuple constructor does.
Secondarily, this fixes an inference bug where it would create Vararg{Union{}} and then incorrectly handle that fieldtype.
There is an upgrade helper for packages that rely on this not throwing an exception of
Base.Iterators.TupleOrBottom(p...), though (judging by changes in Base), I don't expect it to be widely needed. We can do a PkgEval run though to verify.