Fix nospecializing Functions in Union{Nothing,Function} params

[NHDaly]

Fixes #59326.

Change the logic that decides not to specialize a function parameter based on whether or not the supplied argument is a Function, and that function is not used, so that it will still work if the SpecType is a Union{Function,Nothing} or any other union that contains a Function.

The logic is changed from a hardcoded rule of type_i == Function || type_i == Any || type_i == Base.Callable to type_i >: Function.

This covers all of the above cases, but also includes custom Union{Function, T} such as Union{Function, Nothing}.

[NHDaly]

I've added backport labels since I believe this is a bug-fix. If that's wrong we can remove them.

[vtjnash]

LGTM. You also need to update the reverse function:

diff --git a/src/gf.c b/src/gf.c
index 39dd47d64e..4ec67d3ca7 100644
--- a/src/gf.c
+++ b/src/gf.c
@@ -1442,15 +1442,9 @@ JL_DLLEXPORT int jl_isa_compileable_sig(
         int notcalled_func = (i_arg > 0 && i_arg <= 8 && !(definition->called & (1 << (i_arg - 1))) &&
                               !jl_has_free_typevars(decl_i) &&
                               jl_subtype(elt, (jl_value_t*)jl_function_type));
-        if (notcalled_func && (type_i == (jl_value_t*)jl_any_type ||
-                               type_i == (jl_value_t*)jl_function_type ||
-                               (jl_is_uniontype(type_i) && // Base.Callable
-                                ((((jl_uniontype_t*)type_i)->a == (jl_value_t*)jl_function_type &&
-                                  ((jl_uniontype_t*)type_i)->b == (jl_value_t*)jl_type_type) ||
-                                 (((jl_uniontype_t*)type_i)->b == (jl_value_t*)jl_function_type &&
-                                  ((jl_uniontype_t*)type_i)->a == (jl_value_t*)jl_type_type))))) {
-            // and attempt to despecialize types marked Function, Callable, or Any
-            // when called with a subtype of Function but is not called
+        if (notcalled_func && jl_subtype((jl_value_t*)jl_function_type, type_i)) {
+            // and attempt to despecialize types marked as a supertype of Function (i.e.
+            // Function, Callable, Any, or a Union{Function, T})
             if (elt == (jl_value_t*)jl_function_type)
                 continue;
             JL_GC_POP();

[vtjnash]

It is a bit too much of a change to alter heuristics for older versions, but it can apply to v1.12

[JeffBezanson]

This is fine but I think, looking forward, we should consider actually removing this heuristic. I just tried a build with that change for fun, and so far I don't see serious problems. The sysimage is not even much bigger.

[NHDaly]

oh nice, thanks Jameson! 👍 I've pushed up that change as well. Should be good to merge once tests pass.

[NHDaly]

This is fine but I think, looking forward, we should consider actually removing this heuristic. I just tried a build with that change for fun, and so far I don't see serious problems. The sysimage is not even much bigger.

Interesting, @JeffBezanson! 🤔 What would be the benefit of that specialization? I think this heuristic does make sense to me: if you're not going to call the function, it's really just data you're passing through, so why specialize on it?

[JeffBezanson]

Usually the problem is that the function argument is passed along to another function that does call it, but the intermediate function inserts a dispatch barrier.

When this heuristic was introduced we did not even have @nospecialize (I was hoping we wouldn't need it but oh well!!), and sprinkling that around plus other latency improvements may have made this obsolete.