RFC: Teach intersection that 4 != 5 #39098
Conversation
vtjnash
added
the
needs pkgeval
|
I tried rebasing, but noted it currently has this particularly egregious error (both before and after rebasing): |
This one is very tricky, because we need to represent
"Vararg with at least n-elements", which we don't really
have a representation for. This just normalizes it to
Tuple{Vararg{T, N}} and stores an additional constraint
out of line that `N` must be greater or equal `n`. Then
if `N` is not resolved to an integer, we will normalize
this in a post processing step to e.g. `Tuple{T, T, Vararg{T, N}}`
(for `n == 2`). Eventually it might make sense to give `Vararg`
itself a lower bound. For now this seems to work ok, but
this code is very tricky, so we should PkgEval it before
merging.
| if (vb->offset < 0) { | ||
| // Insert additional copies of the type to make up the | ||
| // `offset` difference. | ||
| *params = jl_svec_copy_resize(*params, jl_svec_len(*params) - vb->offset); | ||
| for (size_t i = 0; i < -vb->offset; ++i) { | ||
| jl_svecset(*params, pos + i, ii->T); | ||
| } | ||
| jl_svecset(*params, pos + -vb->offset, ii); | ||
| } |
There was a problem hiding this comment.
There seems no reason to do this.
If vb->offset < 0, these parameters should have been intersected during intersect_tuple.
A simple example
julia> A = Tuple{Tuple{T,Vararg{T,N}},NTuple{N,T}} where {N,T}
Tuple{Tuple{T, Vararg{T, N}}, Tuple{Vararg{T, N}}} where {N, T}
julia> T = Tuple{Tuple,NTuple{4,Int}} where {M}
Tuple{Tuple, NTuple{4, Int64}}
julia> typeintersect(A, T)
Tuple{NTuple{6, Int64}, NTuple{4, Int64}}The above example wont be fixed after removing this branch, but that should be a bug during re-intersection. (see 4dcce49)
My local log shows this PR gives wrong result within the 1st round intersection.
There was a problem hiding this comment.
Another bad regression.
julia> A = Tuple{Tuple{T,Vararg{T,N}},NTuple{N,T}} where {N,T}
Tuple{Tuple{T, Vararg{T, N}}, Tuple{Vararg{T, N}}} where {N, T}
julia> T = Tuple{Tuple,NTuple{M,Int}} where {M}
Tuple{Tuple, Tuple{Vararg{Int64, M}}} where M
julia> typeintersect(A, T)
Tuple{Tuple{Int64, Int64, Vararg{Int64, N}}, Tuple{}} where NThe first looks similar as the above example.
There was a problem hiding this comment.
thanks for looking, I started this before I noticed your PR, and was looking at getting that merged instead, then to revisit whether this fixed any additional cases
|
Looks like all examples here are fixed now. And #46604 exists for continued accuracy improvements that might be possible. |
This one is very tricky, because we need to represent
"Vararg with at least n-elements", which we don't really
have a representation for. This just normalizes it to
Tuple{Vararg{T, N}} and stores an additional constraint
out of line that
Nmust be greater or equaln. Thenif
Nis not resolved to an integer, we will normalizethis in a post processing step to e.g.
Tuple{T, T, Vararg{T, N}}(for
n == 2). Eventually it might make sense to giveVarargitself a lower bound. For now this seems to work ok, but
this code is very tricky, so we should PkgEval it before
merging.
Fix #39088