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

Fix some incorrect piracy flagging #131

Merged
merged 1 commit into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
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
32 changes: 31 additions & 1 deletion src/piracy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,37 @@ is_foreign(@nospecialize(T::TypeVar), pkg::Base.PkgId) = is_foreign(T.ub, pkg)
end

function is_foreign(@nospecialize(U::Union), pkg::Base.PkgId)
# Even if Foo is local, overloading f(::Union{Foo, Int}) with foreign f
# Even if Foo is local, overloading f(::Union{Foo, Int}) with foreign f
# is piracy.
any(T -> is_foreign(T, pkg), Base.uniontypes(U))
end

function is_foreign_method(@nospecialize(U::Union), pkg::Base.PkgId)
# When installing a method for a union type, then we only consider it as
# foreign if *all* parameters of the union are foreign, i.e. overloading
# Union{Foo, Int}() is not piracy.
all(T -> is_foreign(T, pkg), Base.uniontypes(U))
end

function is_foreign_method(@nospecialize(x::Any), pkg::Base.PkgId)
is_foreign(x, pkg)
end

function is_foreign_method(@nospecialize(T::DataType), pkg::Base.PkgId)
params = T.parameters
# For Type{Foo}, we consider it to originate from the same as Foo
C = getfield(parentmodule(T), nameof(T))
if C === Type
@assert length(params) == 1
U = first(params)
return is_foreign_method(first(params), pkg)
end

# fallback to general code
return is_foreign(T, pkg)
end


function is_pirate(meth::Method)
method_pkg = Base.PkgId(meth.module)

Expand All @@ -123,6 +149,10 @@ function is_pirate(meth::Method)
signature = signature.body
end

# the first parameter in the signature is the function type, and it
# follows slightly other rules if it happens to be a Union type
is_foreign_method(signature.parameters[1], method_pkg) || return false

all(param -> is_foreign(param, method_pkg), signature.parameters)
Comment on lines +154 to 156
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the first entry of signature.parameters is handled differently, the is_foreign check should only be applied to signature.parameters[2:end], right?

end

Expand Down
9 changes: 8 additions & 1 deletion test/test_piracy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ Base.findlast(::Type{Val{Foo}}, x::Int) = x + 1
Base.findlast(::Tuple{Vararg{Bar{Set{Int}}}}, x::Int) = x + 1
Base.findlast(::Val{:foo}, x::Int) = x + 1

# Not piracy
const MyUnion = Union{Int,Foo}
MyUnion(x::Int) = x

export MyUnion

# Piracy
Base.findfirst(::Set{Vector{Char}}, ::Int) = 1
Base.findfirst(::Union{Foo,Bar{Set{Unsigned}},UInt}, ::Tuple{Vararg{String}}) = 1
Expand All @@ -48,9 +54,10 @@ end

# 2 Foo constructors
# 2 from f
# 1 from MyUnion
# 5 from findlast
# 3 from findfirst
@test length(meths) == 2 + 2 + 5 + 3
@test length(meths) == 2 + 2 + 1 + 5 + 3

# Test what is foreign
BasePkg = Base.PkgId(Base)
Expand Down