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

ambiguous: fix a rare case of comparison #48196

Merged
merged 1 commit into from
Jan 10, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1789,7 +1789,9 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
end
end
# if ml-matches reported the existence of an ambiguity over their
# intersection, see if both m1 and m2 may be involved in it
# intersection, see if both m1 and m2 seem to be involved in it
# (if one was fully dominated by a different method, we want to will
# report the other ambiguous pair)
have_m1 = have_m2 = false
for match in ms
match = match::Core.MethodMatch
Expand All @@ -1814,18 +1816,14 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
minmax = m
end
end
if minmax === nothing
if minmax === nothing || minmax == m1 || minmax == m2
return true
end
for match in ms
m = match.method
m === minmax && continue
if match.fully_covers
if !morespecific(minmax.sig, m.sig)
return true
end
else
if morespecific(m.sig, minmax.sig)
if !morespecific(minmax.sig, m.sig)
if match.full_covers || !morespecific(m.sig, minmax.sig)
return true
end
end
Expand All @@ -1842,12 +1840,12 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
if ti2 <: m1.sig && ti2 <: m2.sig
ti = ti2
elseif ti != ti2
# TODO: this would be the correct way to handle this case, but
# TODO: this would be the more correct way to handle this case, but
# people complained so we don't do it
# inner(ti2) || return false
return false # report that the type system failed to decide if it was ambiguous by saying they definitely aren't
#inner(ti2) || return false # report that the type system failed to decide if it was ambiguous by saying they definitely are
return false # report that the type system failed to decide if it was ambiguous by saying they definitely are not
else
return false # report that the type system failed to decide if it was ambiguous by saying they definitely aren't
return false # report that the type system failed to decide if it was ambiguous by saying they definitely are not
end
end
inner(ti) || return false
Expand Down