gf: allow method shadowing with exact signatures without deletion (Ju…

…liaLang#53415)

We already allowed method shadowing for an inexact signature and
implicit method deletion by exact signatures, so it was fairly arbitrary
to implement it as a deletion for a replacement by an exact signature
rather than use the morespecific definition for this.

This change should be useful to Mocking libraries, such as Pluto, which
previously had a buggier version of this which tried to re-activate or
re-define the old methods.

There is a bit of code cleanup in here, either of unused features, or of
aspects that were broken, or of world_valid computations that were not
actually impacting the final result (e.g. when there is a more specific
result matches, it does not need to limit the result valid world range
because of a less specific result that does not get returned).

NEWS.md

@@ -7,6 +7,16 @@ New language features
 Language changes
 ----------------
 
+ - When methods are replaced with exactly equivalent ones, the old method is no
+ longer deleted implicitly simultaneously, although the new method does take
+ priority and become more specific than the old method. Thus if the new
+ method is deleted later, the old method will resume operating. This can be
+ useful to mocking frameworks (such as in SparseArrays, Pluto, and Mocking,
+ among others), as they do not need to explicitly restore the old method.
+ While inference and compilation still must be repeated with this, it also
+ may pave the way for inference to be able to intelligently re-use the old
+ results, once the new method is deleted. ([#53415])
+
 Compiler/Runtime improvements
 -----------------------------
 

base/deprecated.jl

@@ -29,11 +29,11 @@ To prevent `old` from being exported, set `export_old` to `false`.
 
 # Examples
 ```jldoctest
-julia> @deprecate old(x) new(x)
-old (generic function with 1 method)
+julia> @deprecate old_export(x) new(x)
+old_export (generic function with 1 method)
 
-julia> @deprecate old(x) new(x) false
-old (generic function with 1 method)
+julia> @deprecate old_public(x) new(x) false
+old_public (generic function with 1 method)
 ```
 
 Calls to `@deprecate` without explicit type-annotations will define

base/reflection.jl

@@ -887,6 +887,7 @@ A special case where exact behavior is guaranteed: when `T <: S`,
 typeintersect(@nospecialize(a), @nospecialize(b)) = (@_total_meta; ccall(:jl_type_intersection, Any, (Any, Any), a::Type, b::Type))
 
 morespecific(@nospecialize(a), @nospecialize(b)) = (@_total_meta; ccall(:jl_type_morespecific, Cint, (Any, Any), a::Type, b::Type) != 0)
+morespecific(a::Method, b::Method) = ccall(:jl_method_morespecific, Cint, (Any, Any), a, b) != 0
 
 """
  fieldoffset(type, i)
@@ -2492,7 +2493,7 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
  for match in ms
  m = match.method
  match.fully_covers || continue
- if minmax === nothing || morespecific(m.sig, minmax.sig)
+ if minmax === nothing || morespecific(m, minmax)
  minmax = m
  end
  end
@@ -2502,8 +2503,8 @@ function isambiguous(m1::Method, m2::Method; ambiguous_bottom::Bool=false)
  for match in ms
  m = match.method
  m === minmax && continue
- if !morespecific(minmax.sig, m.sig)
- if match.fully_covers || !morespecific(m.sig, minmax.sig)
+ if !morespecific(minmax, m)
+ if match.fully_covers || !morespecific(m, minmax)
  return true
  end
  end

doc/src/devdocs/types.md

@@ -521,10 +521,6 @@ than the other.) Likewise, `Tuple{Int,Vararg{Int}}` is not a subtype of `Tuple{
 considered more specific. However, `morespecific` does get a bonus for length: in particular,
 `Tuple{Int,Int}` is more specific than `Tuple{Int,Vararg{Int}}`.
 
-If you're debugging how methods get sorted, it can be convenient to define the function:
-
-```julia
-type_morespecific(a, b) = ccall(:jl_type_morespecific, Cint, (Any,Any), a, b)
-```
-
-which allows you to test whether tuple type `a` is more specific than tuple type `b`.
+Additionally, if 2 methods are defined with identical signatures, per type-equal, then they
+will instead by compared by order of addition, such that the later method is more specific
+than the earlier one.

doc/src/manual/modules.md

@@ -184,7 +184,6 @@ Stacktrace:
  @ none:0
  [2] top-level scope
  @ none:1
-
 ```
 
 This error prevents accidentally adding methods to functions in other modules that you only intended to use.
@@ -196,17 +195,16 @@ julia> using .NiceStuff
 julia> struct Cat end
 
 julia> NiceStuff.nice(::Cat) = "nice 😸"
-
 ```
 
 Alternatively, you can `import` the specific function name:
 ```jldoctest module_manual
 julia> import .NiceStuff: nice
 
-julia> struct Cat end
+julia> struct Mouse end
 
-julia> nice(::Cat) = "nice 😸"
-nice (generic function with 2 methods)
+julia> nice(::Mouse) = "nice 🐭"
+nice (generic function with 3 methods)
 ```
 
 Which one you choose is a matter of style. The first form makes it clear that you are adding a

src/jl_exported_funcs.inc

@@ -479,7 +479,7 @@
  XX(jl_type_intersection) \
  XX(jl_type_intersection_with_env) \
  XX(jl_type_morespecific) \
- XX(jl_type_morespecific_no_subtype) \
+ XX(jl_method_morespecific) \
  XX(jl_type_union) \
  XX(jl_type_unionall) \
  XX(jl_unbox_bool) \

src/julia.h

@@ -1711,6 +1711,7 @@ JL_DLLEXPORT jl_value_t *jl_type_unionall(jl_tvar_t *v, jl_value_t *body);
 JL_DLLEXPORT const char *jl_typename_str(jl_value_t *v) JL_NOTSAFEPOINT;
 JL_DLLEXPORT const char *jl_typeof_str(jl_value_t *v) JL_NOTSAFEPOINT;
 JL_DLLEXPORT int jl_type_morespecific(jl_value_t *a, jl_value_t *b);
+JL_DLLEXPORT int jl_method_morespecific(jl_method_t *ma, jl_method_t *mb);
 
 STATIC_INLINE int jl_is_dispatch_tupletype(jl_value_t *v) JL_NOTSAFEPOINT
 {

src/julia_internal.h

@@ -1465,8 +1465,6 @@ struct jl_typemap_assoc {
  size_t const world;
  // outputs
  jl_svec_t *env; // subtype env (initialize to null to perform intersection without an environment)
- size_t min_valid;
- size_t max_valid;
 };
 
 jl_typemap_entry_t *jl_typemap_assoc_by_type(

src/subtype.c

@@ -5150,6 +5150,26 @@ JL_DLLEXPORT int jl_type_morespecific_no_subtype(jl_value_t *a, jl_value_t *b)
  return type_morespecific_(a, b, a, b, 0, NULL);
 }
 
+// Equivalent to `jl_type_morespecific` of the signatures, except that more recent
+// methods are more specific, iff the methods signatures are type-equal
+JL_DLLEXPORT int jl_method_morespecific(jl_method_t *ma, jl_method_t *mb)
+{
+ jl_value_t *a = (jl_value_t*)ma->sig;
+ jl_value_t *b = (jl_value_t*)mb->sig;
+ if (obviously_disjoint(a, b, 1))
+ return 0;
+ if (jl_has_free_typevars(a) || jl_has_free_typevars(b))
+ return 0;
+ if (jl_subtype(b, a)) {
+ if (jl_types_equal(a, b))
+ return jl_atomic_load_relaxed(&ma->primary_world) > jl_atomic_load_relaxed(&mb->primary_world);
+ return 0;
+ }
+ if (jl_subtype(a, b))
+ return 1;
+ return type_morespecific_(a, b, a, b, 0, NULL);
+}
+
 #ifdef __cplusplus
 }
 #endif

src/typemap.c

@@ -836,9 +836,7 @@ static jl_typemap_entry_t *jl_typemap_entry_assoc_by_type(
  size_t n = jl_nparams(unw);
  int typesisva = n == 0 ? 0 : jl_is_vararg(jl_tparam(unw, n-1));
  for (; ml != (void*)jl_nothing; ml = jl_atomic_load_relaxed(&ml->next)) {
- if (search->max_valid < jl_atomic_load_relaxed(&ml->min_world))
- continue;
- if (search->min_valid > jl_atomic_load_relaxed(&ml->max_world))
+ if (search->world < jl_atomic_load_relaxed(&ml->min_world) || search->world > jl_atomic_load_relaxed(&ml->max_world))
  continue;
  size_t lensig = jl_nparams(jl_unwrap_unionall((jl_value_t*)ml->sig));
  if (lensig == n || (ml->va && lensig <= n+1)) {
@@ -877,26 +875,7 @@ static jl_typemap_entry_t *jl_typemap_entry_assoc_by_type(
  }
  }
  if (ismatch) {
- size_t min_world = jl_atomic_load_relaxed(&ml->min_world);
- size_t max_world = jl_atomic_load_relaxed(&ml->max_world);
- if (search->world < min_world) {
- // ignore method table entries that are part of a later world
- if (search->max_valid >= min_world)
- search->max_valid = min_world - 1;
- }
- else if (search->world > max_world) {
- // ignore method table entries that have been replaced in the current world
- if (search->min_valid <= max_world)
- search->min_valid = max_world + 1;
- }
- else {
- // intersect the env valid range with method's valid range
- if (search->min_valid < min_world)
- search->min_valid = min_world;
- if (search->max_valid > max_world)
- search->max_valid = max_world;
- return ml;
- }
+ return ml;
  }
  }
  if (resetenv)

stdlib/Serialization/src/Serialization.jl

@@ -1111,7 +1111,7 @@ function deserialize(s::AbstractSerializer, ::Type{Method})
  end
  if !is_for_opaque_closure
  mt = ccall(:jl_method_table_for, Any, (Any,), sig)
- if mt !== nothing && nothing === ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), mt, sig, typemax(UInt))
+ if mt !== nothing && nothing === ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), mt, sig, Base.get_world_counter())
  ccall(:jl_method_table_insert, Cvoid, (Any, Any, Ptr{Cvoid}), mt, meth, C_NULL)
  end
  end

test/compiler/invalidation.jl

@@ -142,7 +142,9 @@ begin take!(GLOBAL_BUFFER)
  # this redefinition below should invalidate the cache of `pr48932_callee` but not that of `pr48932_caller`
  pr48932_callee(x) = (print(GLOBAL_BUFFER, x); nothing)
 
- @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee))))
+ @test length(Base.methods(pr48932_callee)) == 2
+ @test Base.only(Base.methods(pr48932_callee, Tuple{Any})) === first(Base.methods(pr48932_callee))
+ @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee, Tuple{Any}))))
  let mi = only(Base.specializations(Base.only(Base.methods(pr48932_caller))))
  # Base.method_instance(pr48932_callee, (Any,))
  ci = mi.cache
@@ -206,7 +208,7 @@ begin take!(GLOBAL_BUFFER)
  # this redefinition below should invalidate the cache of `pr48932_callee_inferable` but not that of `pr48932_caller_unuse`
  pr48932_callee_inferable(x) = (print(GLOBAL_BUFFER, "foo"); x)
 
- @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee_inferable))))
+ @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee_inferable, Tuple{Any}))))
  let mi = Base.method_instance(pr48932_caller_unuse, (Int,))
  ci = mi.cache
  @test isdefined(ci, :next)
@@ -266,7 +268,7 @@ begin take!(GLOBAL_BUFFER)
  # this redefinition below should invalidate the cache of `pr48932_callee_inlined` but not that of `pr48932_caller_inlined`
  @noinline pr48932_callee_inlined(@nospecialize x) = (print(GLOBAL_BUFFER, x); nothing)
 
- @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee_inlined))))
+ @test isempty(Base.specializations(Base.only(Base.methods(pr48932_callee_inlined, Tuple{Any}))))
  let mi = Base.method_instance(pr48932_caller_inlined, (Int,))
  ci = mi.cache
  @test isdefined(ci, :next)

test/reflection.jl

@@ -474,7 +474,7 @@ fLargeTable() = 4
 fLargeTable(::Union, ::Union) = "a"
 @test fLargeTable(Union{Int, Missing}, Union{Int, Missing}) == "a"
 fLargeTable(::Union, ::Union) = "b"
-@test length(methods(fLargeTable)) == 205
+@test length(methods(fLargeTable)) == 206
 @test fLargeTable(Union{Int, Missing}, Union{Int, Missing}) == "b"
 
 # issue #15280

test/specificity.jl

@@ -1,9 +1,9 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
 function args_morespecific(a, b)
- sp = (ccall(:jl_type_morespecific, Cint, (Any,Any), a, b) != 0)
+ sp = Base.morespecific(a, b)
  if sp # make sure morespecific(a,b) implies !morespecific(b,a)
- @test ccall(:jl_type_morespecific, Cint, (Any,Any), b, a) == 0
+ @test !Base.morespecific(b, a)
  end
  return sp
 end

test/worlds.jl

@@ -454,3 +454,27 @@ let e = ExceptionUnwrapping.X(nothing)
  @test ExceptionUnwrapping.result == [false, true]
  empty!(ExceptionUnwrapping.result)
 end
+
+fshadow() = 1
+gshadow() = fshadow()
+@test fshadow() === 1
+@test gshadow() === 1
+fshadow_m1 = which(fshadow, ())
+fshadow() = 2
+fshadow() = 3
+@test fshadow() === 3
+@test gshadow() === 3
+fshadow_m3 = which(fshadow, ())
+Base.delete_method(fshadow_m1)
+@test fshadow() === 3
+@test gshadow() === 3
+Base.delete_method(fshadow_m3)
+fshadow_m2 = which(fshadow, ())
+@test fshadow() === 2
+@test gshadow() === 2
+Base.delete_method(fshadow_m2)
+@test_throws MethodError(fshadow, (), Base.tls_world_age()) gshadow()
+@test Base.morespecific(fshadow_m3, fshadow_m2)
+@test Base.morespecific(fshadow_m2, fshadow_m1)
+@test Base.morespecific(fshadow_m3, fshadow_m1)
+@test !Base.morespecific(fshadow_m2, fshadow_m3)