Skip to content

Commit

Permalink
Merge pull request #14759 from JuliaLang/jb/overwritten
Browse files Browse the repository at this point in the history
RFC: always warn on overwritten methods
  • Loading branch information
JeffBezanson committed Jan 26, 2016
2 parents f2c5393 + ee15300 commit 84c06b1
Show file tree
Hide file tree
Showing 30 changed files with 79 additions and 94 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ Language changes

* Relational symbols are now allowed as infix operators ([#8036]).

* A warning is always given when a method is overwritten (previously, this was done only when the new
and old definitions were in separate modules) ([#14759]).

Command-line option changes
---------------------------

Expand Down Expand Up @@ -1764,3 +1767,4 @@ Too numerous to mention.
[#14243]: https://github.com/JuliaLang/julia/issues/14243
[#14413]: https://github.com/JuliaLang/julia/issues/14413
[#14424]: https://github.com/JuliaLang/julia/issues/14424
[#14759]: https://github.com/JuliaLang/julia/issues/14759
2 changes: 1 addition & 1 deletion base/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -380,14 +380,14 @@ for (f, cachef, scalarf) in ((:.==, :bitcache_eq , :(==)),
(:.< , :bitcache_lt , :< ),
(:.!=, :bitcache_neq, :!= ),
(:.<=, :bitcache_le , :<= ))
@eval ($cachef)(A::AbstractArray, B::AbstractArray, l::Int, ind::Int, C::Vector{Bool}) = 0
for (sigA, sigB, expA, expB, shape) in ((:Any, :AbstractArray,
:A, :(B[ind]),
:(size(B))),
(:AbstractArray, :Any,
:(A[ind]), :B,
:(size(A))))
@eval begin
($cachef)(A::AbstractArray, B::AbstractArray, l::Int, ind::Int, C::Vector{Bool}) = 0
function ($cachef)(A::$sigA, B::$sigB, l::Int, ind::Int, C::Vector{Bool})
left = l - ind + 1
@inbounds begin
Expand Down
16 changes: 16 additions & 0 deletions base/checked.jl
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,19 @@ end
function checked_neg{T<:UnsignedInt}(x::T)
checked_sub(T(0), x)
end
if BrokenSignedInt != Union{}
function checked_neg{T<:BrokenSignedInt}(x::T)
r = -x
(x<0) & (r<0) && throw(OverflowError())
r
end
end
if BrokenUnsignedInt != Union{}
function checked_neg{T<:BrokenUnsignedInt}(x::T)
x != 0 && throw(OverflowError())
T(0)
end
end

"""
Base.checked_abs(x)
Expand Down Expand Up @@ -124,18 +128,22 @@ end
function checked_add{T<:UnsignedInt}(x::T, y::T)
box(T, checked_uadd_int(unbox(T,x), unbox(T,y)))
end
if BrokenSignedInt != Union{}
function checked_add{T<:BrokenSignedInt}(x::T, y::T)
r = x + y
# x and y have the same sign, and the result has a different sign
(x<0) == (y<0) != (r<0) && throw(OverflowError())
r
end
end
if BrokenUnsignedInt != Union{}
function checked_add{T<:BrokenUnsignedInt}(x::T, y::T)
# x + y > typemax(T)
# Note: ~y == -y-1
x > ~y && throw(OverflowError())
x + y
end
end

# Handle multiple arguments
checked_add(x) = x
Expand Down Expand Up @@ -167,17 +175,21 @@ end
function checked_sub{T<:UnsignedInt}(x::T, y::T)
box(T, checked_usub_int(unbox(T,x), unbox(T,y)))
end
if BrokenSignedInt != Union{}
function checked_sub{T<:BrokenSignedInt}(x::T, y::T)
r = x - y
# x and y have different signs, and the result has a different sign than x
(x<0) != (y<0) == (r<0) && throw(OverflowError())
r
end
end
if BrokenUnsignedInt != Union{}
function checked_sub{T<:BrokenUnsignedInt}(x::T, y::T)
# x - y < 0
x < y && throw(OverflowError())
x - y
end
end

"""
Base.checked_mul(x, y)
Expand All @@ -194,16 +206,20 @@ end
function checked_mul{T<:UnsignedInt}(x::T, y::T)
box(T, checked_umul_int(unbox(T,x), unbox(T,y)))
end
if BrokenSignedIntMul != Union{} && BrokenSignedIntMul != Int128
function checked_mul{T<:BrokenSignedIntMul}(x::T, y::T)
r = widemul(x, y)
r % T != r && throw(OverflowError())
r % T
end
end
if BrokenUnsignedIntMul != Union{} && BrokenUnsignedIntMul != UInt128
function checked_mul{T<:BrokenUnsignedIntMul}(x::T, y::T)
r = widemul(x, y)
r % T != r && throw(OverflowError())
r % T
end
end
if Int128 <: BrokenSignedIntMul
# Avoid BigInt
function checked_mul{T<:Int128}(x::T, y::T)
Expand Down
1 change: 0 additions & 1 deletion base/functors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ call(pred::Predicate, x) = pred.f(x)::Bool
immutable EqX{T} <: Func{1}
x::T
end
EqX{T}(x::T) = EqX{T}(x)

call(f::EqX, y) = f.x == y

Expand Down
1 change: 0 additions & 1 deletion base/libgit2/oid.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

Oid(id::Oid) = id
Oid(ptr::Ptr{Oid}) = unsafe_load(ptr)::Oid
Oid(arr::Vector{UInt8}) = Oid(tuple(arr...))

function Oid(ptr::Ptr{UInt8})
if ptr == C_NULL
Expand Down
2 changes: 1 addition & 1 deletion base/libgit2/repository.jl
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function reset!(repo::GitRepo, obj::GitObject, mode::Cint;
end

function clone(repo_url::AbstractString, repo_path::AbstractString,
clone_opts::CloneOptions = CloneOptions())
clone_opts::CloneOptions)
clone_opts_ref = Ref(clone_opts)
repo_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
@check ccall((:git_clone, :libgit2), Cint,
Expand Down
4 changes: 0 additions & 4 deletions base/libgit2/tree.jl
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ function filemode(te::GitTreeEntry)
return ccall((:git_tree_entry_filemode, :libgit2), Cint, (Ptr{Void},), te.ptr)
end

function filemode(te::GitTreeEntry)
return ccall((:git_tree_entry_filemode, :libgit2), Cint, (Ptr{Void},), te.ptr)
end


function object(repo::GitRepo, te::GitTreeEntry)
obj_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
Expand Down
1 change: 0 additions & 1 deletion base/linalg/qr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ function A_mul_Bc!{T}(A::AbstractMatrix{T},Q::QRPackedQ{T})
end
A
end
A_mul_Bc(A::AbstractTriangular, B::Union{QRCompactWYQ,QRPackedQ}) = A_mul_Bc(full(A), B)
function A_mul_Bc{TA,TB}(A::AbstractArray{TA}, B::Union{QRCompactWYQ{TB},QRPackedQ{TB}})
TAB = promote_type(TA,TB)
BB = convert(AbstractMatrix{TAB}, B)
Expand Down
2 changes: 1 addition & 1 deletion base/linalg/triangular.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,7 @@ for t in (UpperTriangular, UnitUpperTriangular, LowerTriangular, UnitLowerTriang
end
end

for f in (:*, :Ac_mul_B, :At_mul_B, :A_mul_Bc, :A_mul_Bt, :Ac_mul_Bc, :At_mul_Bt, :\, :Ac_ldiv_B, :At_ldiv_B)
for f in (:*, :Ac_mul_B, :At_mul_B, :\, :Ac_ldiv_B, :At_ldiv_B)
@eval begin
($f)(A::AbstractTriangular, B::AbstractTriangular) = ($f)(A, full(B))
end
Expand Down
1 change: 0 additions & 1 deletion base/ordering.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ immutable Perm{O<:Ordering,V<:AbstractVector} <: Ordering
order::O
data::V
end
Perm{O<:Ordering,V<:AbstractVector}(o::O,v::V) = Perm{O,V}(o,v)

lt(o::ForwardOrdering, a, b) = isless(a,b)
lt(o::ReverseOrdering, a, b) = lt(o.fwd,b,a)
Expand Down
2 changes: 1 addition & 1 deletion base/pkg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ installed(pkg::AbstractString) = cd(Entry.installed,pkg)
Prints out a summary of what packages are installed and what version and state they're in.
"""
status(io::IO=STDOUT) = cd(Entry.status,io)
status(pkg::AbstractString = "", io::IO=STDOUT) = cd(Entry.status,io,pkg)
status(pkg::AbstractString, io::IO=STDOUT) = cd(Entry.status,io,pkg)

"""
clone(pkg)
Expand Down
2 changes: 1 addition & 1 deletion base/pkg/resolve/versionweight.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ immutable HierarchicalValue{T}
rest::T
end

HierarchicalValue{T}(v::Vector{T}, rest::T = zero(T)) = HierarchicalValue{T}(v, rest)
HierarchicalValue{T}(v::Vector{T}) = HierarchicalValue{T}(v, zero(T))
HierarchicalValue(T::Type) = HierarchicalValue(T[])

Base.zero{T}(::Type{HierarchicalValue{T}}) = HierarchicalValue(T)
Expand Down
2 changes: 0 additions & 2 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,6 @@ spawn_opts_inherit(stdios::StdIOSet, exitcb::Callback=false, closecb::Callback=f
spawn_opts_inherit(in::Redirectable=RawFD(0), out::Redirectable=RawFD(1), err::Redirectable=RawFD(2), args...) =
(tuple(in,out,err,args...),false,false)

spawn(cmds::AbstractCmd, args...; chain::Nullable{ProcessChain}=Nullable{ProcessChain}()) =
spawn(cmds, spawn_opts_swallow(args...)...; chain=chain)
spawn(cmds::AbstractCmd, args...; chain::Nullable{ProcessChain}=Nullable{ProcessChain}()) =
spawn(cmds, spawn_opts_swallow(args...)...; chain=chain)

Expand Down
7 changes: 0 additions & 7 deletions base/promotion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -239,16 +239,9 @@ muladd{T<:Number}(x::T, y::T, z::T) = x*y+z
<{T<:Real}(x::T, y::T) = no_op_err("<" , T)
<={T<:Real}(x::T, y::T) = no_op_err("<=", T)

div{T<:Real}(x::T, y::T) = no_op_err("div", T)
fld{T<:Real}(x::T, y::T) = no_op_err("fld", T)
cld{T<:Real}(x::T, y::T) = no_op_err("cld", T)
rem{T<:Real}(x::T, y::T) = no_op_err("rem", T)
mod{T<:Real}(x::T, y::T) = no_op_err("mod", T)

mod1{T<:Real}(x::T, y::T) = no_op_err("mod1", T)
rem1{T<:Real}(x::T, y::T) = no_op_err("rem1", T)
fld1{T<:Real}(x::T, y::T) = no_op_err("fld1", T)

min(x::Real) = x
max(x::Real) = x
minmax(x::Real) = (x, x)
Expand Down
2 changes: 0 additions & 2 deletions base/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,6 @@ function _mapreducedim!{T,N}(f, op, R::AbstractArray, A::AbstractArray{T,N})
return R
end

mapreducedim!(f, op, R::AbstractArray, A::AbstractArray) = (_mapreducedim!(f, op, R, A); R)

to_op(op) = op
function to_op(op::Function)
is(op, +) ? AddFun() :
Expand Down
2 changes: 1 addition & 1 deletion base/regex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ function next(itr::RegexMatchIterator, prev_match)
(prev_match, nothing)
end

function eachmatch(re::Regex, str::AbstractString, ovr::Bool=false)
function eachmatch(re::Regex, str::AbstractString, ovr::Bool)
RegexMatchIterator(re,str,ovr)
end

Expand Down
2 changes: 0 additions & 2 deletions base/serialize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -732,8 +732,6 @@ deserialize(s::SerializationState, ::Type{BigFloat}) = parse(BigFloat, deseriali

deserialize(s::SerializationState, ::Type{BigInt}) = get(GMP.tryparse_internal(BigInt, deserialize(s), 62, true))

deserialize(s::SerializationState, ::Type{BigInt}) = get(GMP.tryparse_internal(BigInt, deserialize(s), 62, true))

function deserialize(s::SerializationState, t::Type{Regex})
pattern = deserialize(s)
compile_options = deserialize(s)
Expand Down
1 change: 0 additions & 1 deletion base/sparse/umfpack.jl
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ for Tv in (:Float64, :Complex128), Ti in UmfpackIndexTypes
end
end
end
show_umf_info() = show_umf_info(2.)

function umfpack_report_symbolic(symb::Ptr{Void}, level::Real)
old_prl::Float64 = umf_ctrl[UMFPACK_PRL]
Expand Down
1 change: 0 additions & 1 deletion base/stat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ for f in Symbol[
:isdir
:isblockdev
:isfile
:islink
:issocket
:issetuid
:issetgid
Expand Down
2 changes: 0 additions & 2 deletions base/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,6 @@ function iswritable(io::LibuvStream)
return ccall(:uv_is_writable, Cint, (Ptr{Void},), io.handle) != 0
end

nb_available(stream::LibuvStream) = nb_available(stream.buffer)

lock(s::LibuvStream) = lock(s.lock)
unlock(s::LibuvStream) = unlock(s.lock)

Expand Down
1 change: 0 additions & 1 deletion examples/lru.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ BoundedLRU() = BoundedLRU{Any, Any}()

isempty(lru::LRU) = isempty(lru.q)
length(lru::LRU) = length(lru.q)
haskey(lru::LRU, key) = haskey(lru.ht, key)

## associative ##

Expand Down
12 changes: 6 additions & 6 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1229,18 +1229,18 @@ jl_methlist_t *jl_method_list_insert(jl_methlist_t **pml, jl_tupletype_t *type,
if (((l->tvars==jl_emptysvec) == (tvars==jl_emptysvec)) &&
sigs_eq((jl_value_t*)type, (jl_value_t*)l->sig, 1)) {
// method overwritten
if (check_amb && l->func->linfo && method->linfo &&
(l->func->linfo->module != method->linfo->module)) {
if (check_amb && l->func->linfo && method->linfo) {
jl_module_t *newmod = method->linfo->module;
jl_module_t *oldmod = l->func->linfo->module;
JL_STREAM *s = JL_STDERR;
jl_printf(s, "WARNING: Method definition %s",
jl_symbol_name(method->linfo->name));
jl_static_show_func_sig(s, (jl_value_t*)type);
jl_printf(s, " in module %s",
jl_symbol_name(l->func->linfo->module->name));
jl_printf(s, " in module %s", jl_symbol_name(oldmod->name));
print_func_loc(s, l->func->linfo);
jl_printf(s, " overwritten in module %s",
jl_symbol_name(newmod->name));
jl_printf(s, " overwritten");
if (oldmod != newmod)
jl_printf(s, " in module %s", jl_symbol_name(newmod->name));
print_func_loc(s, method->linfo);
jl_printf(s, ".\n");
}
Expand Down
8 changes: 1 addition & 7 deletions test/ccall.jl
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,6 @@ b = ccall((:test_1, libccalltest), Struct1, (Struct1,), a)
@test !(a === b)
@test b.x == s1.x + 1 && b.y == s1.y - 2

function foos1(s::Struct1)
@test !(s === a)
@test s == a
s
end

ci32 = Complex{Int32}(Int32(10),Int32(31))
ba = ccall((:test_2a, libccalltest), Complex{Int32}, (Complex{Int32},), ci32)
bb = ccall((:test_2b, libccalltest), Complex{Int32}, (Complex{Int32},), ci32)
Expand Down Expand Up @@ -134,7 +128,7 @@ verbose && Libc.flush_cstdio()
verbose && println("Testing cfunction roundtrip: ")
# cfunction roundtrip
for (t,v) in ((Complex{Int32},:ci32),(Complex{Int64},:ci64),
(Complex64,:cf32),(Complex128,:cf64),(Struct1,:s1))
(Complex64,:cf32),(Complex128,:cf64),(Struct1,:s1))
fname = symbol("foo"*string(v))
fname1 = symbol("foo1"*string(v))
@eval begin
Expand Down
Loading

0 comments on commit 84c06b1

Please sign in to comment.