Skip to content

Commit

Permalink
Fix more invalidations from overloading == (#36282)
Browse files Browse the repository at this point in the history
* Improve typing of ProcessGroup.refs

* Add type annotation in stacktrace handling

* Eliminate boxing in REPL

Related to #15276
  • Loading branch information
timholy authored Jun 16, 2020
1 parent cde6268 commit 2749582
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 39 deletions.
18 changes: 14 additions & 4 deletions base/cartesian.jl
Original file line number Diff line number Diff line change
Expand Up @@ -353,17 +353,27 @@ const exprresolve_cond_dict = Dict{Symbol,Function}(:(==) => ==,
:(<) => <, :(>) => >, :(<=) => <=, :(>=) => >=)

function exprresolve_arith(ex::Expr)
if ex.head === :call && haskey(exprresolve_arith_dict, ex.args[1]) && all([isa(ex.args[i], Number) for i = 2:length(ex.args)])
return true, exprresolve_arith_dict[ex.args[1]](ex.args[2:end]...)
if ex.head === :call
callee = ex.args[1]
if isa(callee, Symbol)
if haskey(exprresolve_arith_dict, callee) && all(Bool[isa(ex.args[i], Number) for i = 2:length(ex.args)])
return true, exprresolve_arith_dict[callee](ex.args[2:end]...)
end
end
end
false, 0
end
exprresolve_arith(arg) = false, 0

exprresolve_conditional(b::Bool) = true, b
function exprresolve_conditional(ex::Expr)
if ex.head === :call && ex.args[1] keys(exprresolve_cond_dict) && isa(ex.args[2], Number) && isa(ex.args[3], Number)
return true, exprresolve_cond_dict[ex.args[1]](ex.args[2], ex.args[3])
if ex.head === :call
callee = ex.args[1]
if isa(callee, Symbol)
if callee keys(exprresolve_cond_dict) && isa(ex.args[2], Number) && isa(ex.args[3], Number)
return true, exprresolve_cond_dict[callee](ex.args[2], ex.args[3])
end
end
end
false, false
end
Expand Down
2 changes: 1 addition & 1 deletion base/docs/Docs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ function doc!(__module__::Module, b::Binding, str::DocStr, @nospecialize sig = U
# We allow for docstrings to be updated, but print a warning since it is possible
# that over-writing a docstring *may* have been accidental. The warning
# is suppressed for symbols in Main, for interactive use (#23011).
__module__ == Main || @warn "Replacing docs for `$b :: $sig` in module `$(__module__)`"
__module__ === Main || @warn "Replacing docs for `$b :: $sig` in module `$(__module__)`"
else
# The ordering of docstrings for each Binding is defined by the order in which they
# are initially added. Replacing a specific docstring does not change it's ordering.
Expand Down
4 changes: 2 additions & 2 deletions base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=()
ft = typeof(f)
lines = []
# These functions are special cased to only show if first argument is matched.
special = f in [convert, getindex, setindex!]
special = f === convert || f === getindex || f === setindex!
funcs = Any[(f, arg_types_param)]

# An incorrect call method produces a MethodError for convert.
Expand Down Expand Up @@ -681,7 +681,7 @@ function _simplify_include_frames(trace)
kept_frames = trues(i)
first_ignored = nothing
while i >= 1
frame, _ = trace[i]
frame::StackFrame, _ = trace[i]
mod = parentmodule(frame)
if isnothing(first_ignored)
if mod === Base && frame.func === :_include
Expand Down
2 changes: 1 addition & 1 deletion base/multimedia.jl
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ function display(@nospecialize x)
try
return display(displays[i], x)
catch e
isa(e, MethodError) && e.f in (display, show) ||
isa(e, MethodError) && (e.f === display || e.f === show) ||
rethrow()
end
end
Expand Down
14 changes: 14 additions & 0 deletions stdlib/Distributed/src/Distributed.jl
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ function _require_callback(mod::Base.PkgId)
end
end

const REF_ID = Ref(1)
next_ref_id() = (id = REF_ID[]; REF_ID[] = id+1; id)

struct RRID
whence::Int
id::Int

RRID() = RRID(myid(),next_ref_id())
RRID(whence, id) = new(whence,id)
end

hash(r::RRID, h::UInt) = hash(r.whence, hash(r.id, h))
==(r::RRID, s::RRID) = (r.whence==s.whence && r.id==s.id)

include("clusterserialize.jl")
include("cluster.jl") # cluster setup and management, addprocs
include("messages.jl")
Expand Down
24 changes: 12 additions & 12 deletions stdlib/Distributed/src/cluster.jl
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ function set_worker_state(w, state)
end

function check_worker_state(w::Worker)
if w.state == W_CREATED
if w.state === W_CREATED
if !isclusterlazy()
if PGRP.topology === :all_to_all
# Since higher pids connect with lower pids, the remote worker
Expand Down Expand Up @@ -185,13 +185,13 @@ function exec_conn_func(w::Worker)
end

function wait_for_conn(w)
if w.state == W_CREATED
if w.state === W_CREATED
timeout = worker_timeout() - (time() - w.ct_time)
timeout <= 0 && error("peer $(w.id) has not connected to $(myid())")

@async (sleep(timeout); notify(w.c_state; all=true))
wait(w.c_state)
w.state == W_CREATED && error("peer $(w.id) didn't connect to $(myid()) within $timeout seconds")
w.state === W_CREATED && error("peer $(w.id) didn't connect to $(myid()) within $timeout seconds")
end
nothing
end
Expand Down Expand Up @@ -626,7 +626,7 @@ function create_worker(manager, wconfig)
# require the value of config.connect_at which is set only upon connection completion
for jw in PGRP.workers
if (jw.id != 1) && (jw.id < w.id)
(jw.state == W_CREATED) && wait(jw.c_state)
(jw.state === W_CREATED) && wait(jw.c_state)
push!(join_list, jw)
end
end
Expand All @@ -649,7 +649,7 @@ function create_worker(manager, wconfig)
end

for wl in wlist
(wl.state == W_CREATED) && wait(wl.c_state)
(wl.state === W_CREATED) && wait(wl.c_state)
push!(join_list, wl)
end
end
Expand Down Expand Up @@ -767,7 +767,7 @@ end
mutable struct ProcessGroup
name::AbstractString
workers::Array{Any,1}
refs::Dict # global references
refs::Dict{RRID,Any} # global references
topology::Symbol
lazy::Union{Bool, Nothing}

Expand Down Expand Up @@ -851,7 +851,7 @@ function nprocs()
n = length(PGRP.workers)
# filter out workers in the process of being setup/shutdown.
for jw in PGRP.workers
if !isa(jw, LocalProcess) && (jw.state != W_CONNECTED)
if !isa(jw, LocalProcess) && (jw.state !== W_CONNECTED)
n = n - 1
end
end
Expand Down Expand Up @@ -902,7 +902,7 @@ julia> procs()
function procs()
if myid() == 1 || (PGRP.topology === :all_to_all && !isclusterlazy())
# filter out workers in the process of being setup/shutdown.
return Int[x.id for x in PGRP.workers if isa(x, LocalProcess) || (x.state == W_CONNECTED)]
return Int[x.id for x in PGRP.workers if isa(x, LocalProcess) || (x.state === W_CONNECTED)]
else
return Int[x.id for x in PGRP.workers]
end
Expand All @@ -911,7 +911,7 @@ end
function id_in_procs(id) # faster version of `id in procs()`
if myid() == 1 || (PGRP.topology === :all_to_all && !isclusterlazy())
for x in PGRP.workers
if (x.id::Int) == id && (isa(x, LocalProcess) || (x::Worker).state == W_CONNECTED)
if (x.id::Int) == id && (isa(x, LocalProcess) || (x::Worker).state === W_CONNECTED)
return true
end
end
Expand All @@ -933,7 +933,7 @@ Specifically all workers bound to the same ip-address as `pid` are returned.
"""
function procs(pid::Integer)
if myid() == 1
all_workers = [x for x in PGRP.workers if isa(x, LocalProcess) || (x.state == W_CONNECTED)]
all_workers = [x for x in PGRP.workers if isa(x, LocalProcess) || (x.state === W_CONNECTED)]
if (pid == 1) || (isa(map_pid_wrkr[pid].manager, LocalManager))
Int[x.id for x in filter(w -> (w.id==1) || (isa(w.manager, LocalManager)), all_workers)]
else
Expand Down Expand Up @@ -1040,11 +1040,11 @@ function _rmprocs(pids, waitfor)

start = time_ns()
while (time_ns() - start) < waitfor*1e9
all(w -> w.state == W_TERMINATED, rmprocset) && break
all(w -> w.state === W_TERMINATED, rmprocset) && break
sleep(min(0.1, waitfor - (time_ns() - start)/1e9))
end

unremoved = [wrkr.id for wrkr in filter(w -> w.state != W_TERMINATED, rmprocset)]
unremoved = [wrkr.id for wrkr in filter(w -> w.state !== W_TERMINATED, rmprocset)]
if length(unremoved) > 0
estr = string("rmprocs: pids ", unremoved, " not terminated after ", waitfor, " seconds.")
throw(ErrorException(estr))
Expand Down
12 changes: 0 additions & 12 deletions stdlib/Distributed/src/messages.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,6 @@

abstract type AbstractMsg end

const REF_ID = Ref(1)
next_ref_id() = (id = REF_ID[]; REF_ID[] = id+1; id)

struct RRID
whence::Int
id::Int

RRID() = RRID(myid(),next_ref_id())
RRID(whence, id) = new(whence,id)
end
hash(r::RRID, h::UInt) = hash(r.whence, hash(r.id, h))
==(r::RRID, s::RRID) = (r.whence==s.whence && r.id==s.id)

## Wire format description
#
Expand Down
6 changes: 3 additions & 3 deletions stdlib/LibGit2/src/LibGit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ function fetch(repo::GitRepo; remote::AbstractString="origin",
fo = FetchOptions(callbacks=remote_callbacks)
fetch(rmt, refspecs, msg="from $(url(rmt))", options=fo)
catch err
if isa(err, GitError) && err.code == Error.EAUTH
if isa(err, GitError) && err.code === Error.EAUTH
reject(cred_payload)
else
Base.shred!(cred_payload)
Expand Down Expand Up @@ -345,7 +345,7 @@ function push(repo::GitRepo; remote::AbstractString="origin",
push_opts = PushOptions(callbacks=remote_callbacks)
push(rmt, refspecs, force=force, options=push_opts)
catch err
if isa(err, GitError) && err.code == Error.EAUTH
if isa(err, GitError) && err.code === Error.EAUTH
reject(cred_payload)
else
Base.shred!(cred_payload)
Expand Down Expand Up @@ -579,7 +579,7 @@ function clone(repo_url::AbstractString, repo_path::AbstractString;
repo = try
clone(repo_url, repo_path, clone_opts)
catch err
if isa(err, GitError) && err.code == Error.EAUTH
if isa(err, GitError) && err.code === Error.EAUTH
reject(cred_payload)
else
Base.shred!(cred_payload)
Expand Down
2 changes: 1 addition & 1 deletion stdlib/LibGit2/src/rebase.jl
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function commit(rb::GitRebase, sig::GitSignature)
oid_ptr, rb.ptr, C_NULL, sig.ptr, C_NULL, C_NULL)
catch err
# TODO: return current HEAD instead
err.code == Error.EAPPLIED && return nothing
err.code === Error.EAPPLIED && return nothing
rethrow()
end
return oid_ptr[]
Expand Down
8 changes: 5 additions & 3 deletions stdlib/REPL/src/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ function complete_symbol(sym, ffunc, context_module=Main)::Vector{Completion}
# We will exclude the results that the user does not want, as well
# as excluding Main.Main.Main, etc., because that's most likely not what
# the user wants
p = s->(!Base.isdeprecated(mod, s) && s != nameof(mod) && ffunc(mod, s))
p = let mod=mod, modname=nameof(mod)
s->(!Base.isdeprecated(mod, s) && s != modname && ffunc(mod, s))
end
# Looking for a binding in a module
if mod == context_module
# Also look in modules we got through `using`
Expand Down Expand Up @@ -624,9 +626,9 @@ function completions(string, pos, context_module=Main)::Completions
ex = Meta.parse(s * ")", raise=false, depwarn=false)

if isa(ex, Expr)
if ex.head==:call
if ex.head === :call
return complete_methods(ex, context_module), first(frange):method_name_end, false
elseif ex.head==:. && ex.args[2] isa Expr && ex.args[2].head==:tuple
elseif ex.head === :. && ex.args[2] isa Expr && (ex.args[2]::Expr).head === :tuple
return complete_methods(ex, context_module), first(frange):(method_name_end - 1), false
end
end
Expand Down

0 comments on commit 2749582

Please sign in to comment.