-
-
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
Subtype: Fix some diagonal rule related false alarm #53034
Conversation
This decreases the number of intransitive triplets found in the experiment from 6035 to 5123. Here's a sample of those which are not fixed with this PR: (Tuple{Any, X} where X<:A, Tuple{X, X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{Any}} where X>:Any, Tuple{X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{A}} where X<:Union{}, Tuple{X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{X}} where X<:A, Tuple{X, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, C, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{Any}} where A<:X<:A, Tuple{Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, Any, Vararg{A}}, Tuple{X, Any, Vararg{A}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{A}} where A<:X<:A, Tuple{X, A, Vararg{A}} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{A}} where X, Tuple{X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, Any, Vararg{A}}, Tuple{X, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X} where X<:Union{}, Tuple{X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X, Vararg{Any}} where A<:X<:A, Tuple{X, Any, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X} where C<:X<:A, Tuple{X, Any, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{X}} where X<:A, Tuple{X, Any, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X} where X<:C, Tuple{X, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{A}} where X>:Any, Tuple{Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X} where C<:X<:A, Tuple{X, Any} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{X}} where X, Tuple{X, Any, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{X}} where X<:Union{}, Tuple{X, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{X}} where X<:C, Tuple{X, Any, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{A}}, Tuple{X, A, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{X}} where X<:A, Tuple{X, X, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, Any, Vararg{A}}, Tuple{X, Any, Vararg{A}} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X, Vararg{C}} where C<:X<:C, Tuple{X, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, A, Vararg{X}} where X<:A, Tuple{X, A, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, Any, Vararg{C}}, Tuple, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X} where A<:X<:A, Tuple{X, X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, X, Vararg{X}} where C<:X<:A, Tuple{X, Any, Vararg{A}} where A<:X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X, Vararg{X}} where X<:C, Tuple{X, X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X} where X>:Any, Tuple{X, Any} where X>:A, Tuple{Vararg{X}} where X>:A)
(Tuple{A, A, Vararg{X}} where X>:A, Tuple{X, Any, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A)
(Tuple{Any, X} where X>:C, Tuple{X, Any, Vararg{A}} where X>:C, Tuple{Vararg{X}} where X>:A) Here, as in the issue, |
Not surprising to me though. The remaining might no be caused by diagonal check. |
current CI failure is related, though seems like a test issue
|
src/jltypes.c
Outdated
if (jl_is_typevar(a)) { | ||
jl_value_t *na = ((jl_tvar_t*)a)->ub; | ||
hasfree &= jl_has_free_typevars(na); | ||
return simple_subtype(na, b, hasfree); |
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.
In Tuple{Union{Integer,T}, T} where T
, would this cause the result to become Tuple{T, T} where T
, instead of the correct result with a switched Union:
julia> (Union{Tuple{Integer,T}, Tuple{T, T}} where T) == (Tuple{Union{Integer,T}, T} where T)
true
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.
Good catch! That would happen only when T has a lb >: Integer, which also relax the diagonal rule. So it should be fine for this example. But if we replace Integer with a leaf type then diagonal rule still holds, so the next branch need a non leaf check.
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.
Well it turns out this branch might affect method definition.
On master
julia> f(x::Union{AbstractArray,T}) where {T<:Vector} = T
f (generic function with 1 method)
julia> f([1])
ERROR: UndefVarError: `T` not defined
Stacktrace:
[1] f(x::Vector{Int64})
@ Main .\REPL[3]:1
[2] top-level scope
@ REPL[4]:1
This PR
julia> f(x::Union{AbstractArray,T}) where {T<:Vector} = T
WARNING: method definition for f at REPL[1]:1 declares type variable T but does not use it.
f (generic function with 1 method)
julia> f([1])
ERROR: UndefVarError: `T` not defined in static parameter matching
Suggestion: run Test.detect_unbound_args to detect method arguments that do not fully constrain a type parameter.
Stacktrace:
[1] f(x::Vector{Int64})
@ Main .\REPL[1]:1
[2] top-level scope
@ REPL[2]:1
Not sure if this is a regression.
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.
Not sure if this is a regression.
IMO this is an improvement, not a regression, because it's better to throw than be ambiguous.
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.e., you may have fixed a correctness bug?
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.
@N5N3 That is an interesting observation. It is sort of awkward that whether we catch that issue at definition or runtime depends somewhat on what normalizations are implemented for Union. I think I am okay with that change however. It seems fairly unlikely packages would have written that, and the solution is quite simple (just don't write that TypeVar in that place to fix the warning). In particular, it was already doing a similar normalization in several cases, so it is at least not an unexpected new behavior:
(on master)
julia> Union{Integer,T} where T<:Integer
Union{Integer, T} where T<:Integer
julia> Union{Any,T} where T<:Integer
Any # not a Union or TypeVar
julia> Union{Int64, T} where T<:Integer
Union{Int64, T} where T<:Integer
julia> Union{Union{},T} where T<:Integer
Integery # not a Union or TypeVar
No, this was not fixing a correctness bug. There is no ambiguity there.
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.
Unrelatedly, this PR is now very close to the unrelated issue encountered in #51637 (comment), where it may error when the normalization fails, instead of simply not doing this normalization, if you are interested in taking a look at how it can avoid throwing the exception from normalization in the example there (for a later PR).
This PR introduces some new counterexamples to the expected transitivity of Julia script abstract type A end
struct C <: A end
triplets = [
(Tuple{A, X, Vararg{X}} where X<:C, Tuple{A, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:C, Tuple{A, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:C, Tuple{Any, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:C, Tuple{Any, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:Union{}, Tuple{A, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:Union{}, Tuple{A, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:Union{}, Tuple{Any, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, X, Vararg{X}} where X<:Union{}, Tuple{Any, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{Any, X, Vararg{X}} where X<:C, Tuple{Any, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{Any, X, Vararg{X}} where X<:C, Tuple{Any, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{Any, X, Vararg{X}} where X<:Union{}, Tuple{Any, Vararg{X}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{Any, X, Vararg{X}} where X<:Union{}, Tuple{Any, Vararg{X}} where X>:C, Tuple{Vararg{X}} where X>:A),
]
f(t) = t[1] <: t[2] <: t[3]
g(t) = t[1] <: t[3]
println(f.(triplets))
println(g.(triplets)) Results:
|
Will this allow #48205 to be merged safely, or at least fix the first part of Jameson's concerns (xref: #48205 (comment))? |
ab2aefd
to
443e135
Compare
We do more normalization after replacing |
@nsajko These new counterexamples seems fine. |
Union
and simple_union
Nice! However I still get 3815 counterexamples (with the version of the experiment code as available on Gitlab): counterexamples.jl.txt EDIT: to clarify, the same experiment for Julia Inline sample, with just the initial few counterexamples: abstract type A end
struct C <: A end
triplets = [
(Tuple{A, A, Any, Vararg{Any}}, Tuple, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, A, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, A, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{A, X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, A, Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, A, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, A, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, Any, Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, Vararg{Any}}, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{Any, X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, A, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Any, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where C<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where X, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{Any}} where X>:C, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, X, Vararg{Any}} where A<:X<:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, X, Vararg{Any}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, X, Vararg{Any}} where X>:Any, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{Any}}, Tuple{X, X, Vararg{X}} where X>:A, Tuple{Vararg{X}} where X>:A),
(Tuple{A, A, Any, Vararg{A}}, Tuple, Tuple{Vararg{X}} where X>:A),
]
f(t) = t[1] <: t[2] <: t[3]
g(t) = t[1] <: t[3]
function d(v)
allequal(v) || error("not all elements are equal")
first(v)
end
println(d(f.(triplets)))
println(d(g.(triplets))) |
Opps I didn't update the test code. |
close #33137
close #53021