Skip to content

Commit 50eb89b

Browse files
committed
Improve struct revision handling with retry mechanism
Add retry logic for methods that fail to re-evaluate when struct definitions change. Instead of immediately failing, these methods are queued in `pending_eval_sigs` and retried in subsequent revise() calls. Also enhance method detection with `hastype_by_name()` to catch methods using old world-age versions of types by comparing TypeName module and name rather than identity. We may want to unify `hastype` and `hastype_by_name`.
1 parent e704a04 commit 50eb89b

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

src/packagedef.jl

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ const __bpart__ = Base.VERSION >= v"1.12.0-DEV.2047"
157157
# Julia 1.12+: when bindings switch to a new type, we need to re-evaluate method
158158
# definitions using the new binding resolution.
159159
const reeval_methods = Set{Method}()
160+
# Track (signature, module, method) tuples for methods that failed to re-evaluate
161+
# due to type incompatibility and need retry when types become compatible again
162+
const pending_eval_sigs = Set{Tuple{MethodInfoKey, Module, Method}}()
160163

161164
"""
162165
Revise.NOPACKAGE
@@ -889,6 +892,26 @@ function revise(; throw::Bool=false)
889892
queue_errors[(pkgdata, file)] = (err, catch_backtrace())
890893
end
891894
end
895+
# Retry methods that previously failed to re-evaluate
896+
if !isempty(pending_eval_sigs)
897+
for (mt_sig, mod, m) in collect(pending_eval_sigs)
898+
methinfo = get(CodeTracking.method_info, mt_sig, nothing)
899+
if methinfo !== nothing && length(methinfo) == 1
900+
_, ex = methinfo[1]
901+
try
902+
invokelatest(eval_with_signatures, mod, ex; mode=:eval)
903+
# If successful, remove from both queues
904+
delete!(CodeTracking.method_info, mt_sig)
905+
delete!(pending_eval_sigs, (mt_sig, mod, m))
906+
with_logger(_debug_logger) do
907+
@debug "RetrySucceeded" _group="Action" time=time() deltainfo=(mod, ex)
908+
end
909+
catch
910+
# Keep in queue for later
911+
end
912+
end
913+
end
914+
end
892915
# Handle binding invalidations
893916
if !isempty(reeval_methods)
894917
handled = Base.IdSet{Type}()
@@ -923,10 +946,17 @@ function revise(; throw::Bool=false)
923946
!iszero(m.dispatch_status) && with_logger(_debug_logger) do
924947
@debug "DeleteMethod" _group="Action" time=time() deltainfo=(m.sig, MethodSummary(m))
925948
Base.delete_method(m) # ensure that "old data" doesn't get run with "old methods"
926-
delete!(CodeTracking.method_info, mt_sig)
927949
_, ex = methinfo[1]
928-
@debug "Eval" _group="Action" time=time() deltainfo=(mod, ex)
929-
invokelatest(eval_with_signatures, m.module, ex; mode=:eval)
950+
@debug "Eval" _group="Action" time=time() deltainfo=(m.module, ex)
951+
try
952+
invokelatest(eval_with_signatures, m.module, ex; mode=:eval)
953+
delete!(CodeTracking.method_info, mt_sig)
954+
catch err
955+
# Re-evaluation failed, likely due to type incompatibility
956+
# Store in pending_eval_sigs for retry when types become compatible
957+
push!(pending_eval_sigs, (mt_sig, m.module, m))
958+
@debug "EvalFailed" _group="Action" time=time() deltainfo=(m.module, ex, err)
959+
end
930960
end
931961
push!(handled, m.sig)
932962
end

src/visit.jl

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ function methods_with(@nospecialize(T::Type))
77
# condition commented out due to https://github.com/timholy/Revise.jl/pull/894#issuecomment-3274102493
88
# see the "MoreConstructors" test case in test/runtests.jl
99
# if method.module !== Tname.module || method.name !== Tname.name # skip constructor
10-
hastype(method.sig, T) && push!(meths, method)
10+
if hastype(method.sig, T) || hastype_by_name(method.sig, Tname)
11+
push!(meths, method)
12+
end
1113
# end
1214
end
1315
return meths
@@ -44,3 +46,27 @@ function hassubtype(@nospecialize(S), @nospecialize(T))
4446
end
4547
return false
4648
end
49+
50+
# Check if a type signature S contains a reference to a type with the given TypeName
51+
# This is useful for finding methods that reference old world-age versions of a type
52+
function hastype_by_name(@nospecialize(S), Tname::Core.TypeName)
53+
isa(S, TypeVar) && return hastype_by_name(S.ub, Tname)
54+
isa(S, Type) || return false
55+
S_unwrapped = Base.unwrap_unionall(S)
56+
isa(S_unwrapped, Core.TypeofBottom) && return false
57+
if isa(S_unwrapped, Union)
58+
return hastype_by_name(S_unwrapped.a, Tname) | hastype_by_name(S_unwrapped.b, Tname)
59+
end
60+
Base.isvarargtype(S_unwrapped) && return hastype_by_name(S_unwrapped.T, Tname)
61+
if isa(S_unwrapped, DataType)
62+
# Compare TypeNames by their module and name, not by identity (===)
63+
# This is necessary because different world-age versions of a type have different TypeName objects
64+
if S_unwrapped.name.module === Tname.module && S_unwrapped.name.name === Tname.name
65+
return true
66+
end
67+
for P in S_unwrapped.parameters
68+
hastype_by_name(P, Tname) && return true
69+
end
70+
end
71+
return false
72+
end

0 commit comments

Comments
 (0)