Skip to content

Commit 3f1328e

Browse files
committed
use subtyping instead of structural inspection for ambiguity exclusions
Should be slightly more robust, since subtyping already implements these queries with much more finesse than is going to be true of a re-implementation of the query here.
1 parent 3e90d04 commit 3f1328e

File tree

5 files changed

+29
-57
lines changed

5 files changed

+29
-57
lines changed

src/ambiguities.jl

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -154,37 +154,6 @@ function reprpkgid(pkg::PkgId)
154154
return "Base.PkgId(Base.UUID($(repr(uuid.value))), $(repr(name)))"
155155
end
156156

157-
struct _NoValue end
158-
159-
function getobj(m::Method)
160-
signature = Base.unwrap_unionall(m.sig)
161-
ty = if is_kwcall(signature)
162-
signature.parameters[3]
163-
else
164-
signature.parameters[1]
165-
end
166-
ty = Base.unwrap_unionall(ty)
167-
if ty <: Function
168-
try
169-
return ty.instance # this should work for functions
170-
catch
171-
end
172-
end
173-
try
174-
if ty.name.wrapper === Type
175-
return ty.parameters[1]
176-
else
177-
return ty.name.wrapper
178-
end
179-
catch err
180-
@error(
181-
"Failed to obtain a function from `Method`.",
182-
exception = (err, catch_backtrace())
183-
)
184-
end
185-
return _NoValue()
186-
end
187-
188157
function test_ambiguities_impl(
189158
packages::Vector{PkgId},
190159
options::NamedTuple,
@@ -196,9 +165,26 @@ function test_ambiguities_impl(
196165
ambiguities = detect_ambiguities(modules...; options...)
197166

198167
if !isempty(exspecs)
199-
exclude_objs = getobj.(exspecs)
168+
exclude_ft = Any[getobj(spec) for spec in exspecs]
169+
exclude_sig = Any[]
170+
for ft in exclude_ft
171+
if ft isa Type
172+
push!(exclude_sig, Tuple{ft, Vararg})
173+
push!(exclude_sig, Tuple{Core.kwftype(ft), Any, ft, Vararg})
174+
ft = Type{<:ft} # alternatively, Type{ft}
175+
else
176+
ft = typeof(ft)
177+
end
178+
push!(exclude_sig, Tuple{ft, Vararg})
179+
push!(exclude_sig, Tuple{Core.kwftype(ft), Any, ft, Vararg})
180+
end
200181
ambiguities = filter(ambiguities) do (m1, m2)
201-
getobj(m1) exclude_objs && getobj(m2) exclude_objs
182+
for excl in exclude_sig
183+
if m1.sig <: excl || m2.sig <: excl
184+
return false
185+
end
186+
end
187+
return true
202188
end
203189
end
204190

src/piracies.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ function is_pirate(meth::Method; treat_as_own = Union{Function,Type}[])
168168
signature = Base.unwrap_unionall(meth.sig)
169169

170170
function_type_index = 1
171-
if is_kwcall(signature)
171+
if is_kwcall(meth.sig)
172172
# kwcall is a special case, since it is not a real function
173173
# but a wrapper around a function, the third parameter is the original
174174
# function, its positional arguments follow.

src/utils.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ function checked_repr(obj)
5555
return code
5656
end
5757

58-
function is_kwcall(signature::DataType)
58+
function is_kwcall(signature::Type)
5959
@static if VERSION < v"1.9"
60+
signature = Base.unwrap_unionall(signature)::DataType
6061
try
6162
length(signature.parameters) >= 3 || return false
6263
signature <: Tuple{Function,Any,Any,Vararg} || return false
@@ -69,6 +70,6 @@ function is_kwcall(signature::DataType)
6970
return false
7071
end
7172
else
72-
return signature.parameters[1] === typeof(Core.kwcall)
73+
return signature <: Tuple{typeof(Core.kwcall), Any, Any, Vararg}
7374
end
7475
end

test/test_ambiguities.jl

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ include("preamble.jl")
3030
if broken
3131
@test_broken num_ambiguities_ == num_ambiguities
3232
else
33+
if num_ambiguities_ != num_ambiguities
34+
@show exclude
35+
println(strout)
36+
println(strerr)
37+
end
3338
@test num_ambiguities_ == num_ambiguities
3439
end
3540
end
@@ -50,7 +55,7 @@ include("preamble.jl")
5055
check_testcase([PkgWithAmbiguities.ConcreteType], total - num_ambs_ConcreteType)
5156

5257
# exclude abstract supertype without callables and constructors
53-
check_testcase([PkgWithAmbiguities.AbstractType], total)
58+
check_testcase([PkgWithAmbiguities.AbstractType], total - num_ambs_SingletonType - num_ambs_ConcreteType)
5459

5560
@static if VERSION >= v"1.3-"
5661
# for ambiguities between abstract and concrete type callables, only one needs to be excluded

test/test_getobj.jl

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)