Fix nospecializing Functions in `Union{Nothing,Function}` params by NHDaly · Pull Request #59327 · JuliaLang/julia

NHDaly · 2025-08-18T21:07:55Z

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}.

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` to `type_i >: Function`.

NHDaly · 2025-08-19T14:05:51Z

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 · 2025-08-19T17:28:48Z

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

JeffBezanson · 2025-08-20T01:06:57Z

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.

test/core.jl

Co-authored-by: Nick Robinson <npr251@gmail.com>

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

NHDaly · 2025-08-21T16:08:51Z

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

NHDaly · 2025-08-21T16:09:44Z

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 · 2025-08-21T21:43:11Z

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.

…iaLang#59327) Fixes JuliaLang#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}`. --------- Co-authored-by: Nick Robinson <npr251@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com>

…iaLang#59327) (#251) Fixes JuliaLang#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}`. --------- Co-authored-by: Nick Robinson <npr251@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com>

) 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}`. --------- Co-authored-by: Nick Robinson <npr251@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com> (cherry picked from commit 9612115)

NHDaly added 2 commits August 18, 2025 15:00

Cleanup & improve comment

f12a211

NHDaly force-pushed the nhd-function-nospecialize-59326 branch from 1cc730c to 8fdcee9 Compare August 18, 2025 22:29

Add tests for (lack of) specialization

19250e0

NHDaly force-pushed the nhd-function-nospecialize-59326 branch from 8fdcee9 to 19250e0 Compare August 19, 2025 14:04

NHDaly added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Aug 19, 2025

vtjnash reviewed Aug 19, 2025

View reviewed changes

vtjnash removed backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 labels Aug 19, 2025

KristofferC mentioned this pull request Aug 19, 2025

Backports for 1.12-rc2 #59337

Merged

27 tasks

nickrobinson251 reviewed Aug 21, 2025

View reviewed changes

test/core.jl Outdated Show resolved Hide resolved

NHDaly and others added 3 commits August 21, 2025 09:30

Fix typo in test

a917f11

Co-authored-by: Nick Robinson <npr251@gmail.com>

PR feedback: Make the same change in jl_isa_compileable_sig.

7ddbbee

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Merge branch 'master' into nhd-function-nospecialize-59326

ba9c09a

NHDaly requested a review from vtjnash August 21, 2025 17:28

NHDaly merged commit 9612115 into master Aug 21, 2025
7 of 8 checks passed

NHDaly deleted the nhd-function-nospecialize-59326 branch August 21, 2025 22:04

NHDaly mentioned this pull request Aug 21, 2025

Fix nospecializing Functions in Union{Nothing,Function} params (#59327) RelationalAI/julia#251

Merged

3 tasks

KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Sep 11, 2025

KristofferC mentioned this pull request Nov 20, 2025

set VERSION to 1.12.2 #60182

Merged

KristofferC mentioned this pull request Dec 12, 2025

release-1.12: Revert "build: More msys2 fixes (#59028)" #60374

Merged

KristofferC mentioned this pull request Jan 3, 2026

set VERSION to 1.12.4 #60539

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix nospecializing Functions in `Union{Nothing,Function}` params#59327

Fix nospecializing Functions in `Union{Nothing,Function}` params#59327
NHDaly merged 6 commits intomasterfrom
nhd-function-nospecialize-59326

NHDaly commented Aug 18, 2025

Uh oh!

NHDaly commented Aug 19, 2025

Uh oh!

vtjnash left a comment

Uh oh!

vtjnash commented Aug 19, 2025

Uh oh!

JeffBezanson commented Aug 20, 2025

Uh oh!

Uh oh!

NHDaly commented Aug 21, 2025

Uh oh!

NHDaly commented Aug 21, 2025

Uh oh!

JeffBezanson commented Aug 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

NHDaly commented Aug 18, 2025

Uh oh!

NHDaly commented Aug 19, 2025

Uh oh!

vtjnash left a comment

Choose a reason for hiding this comment

Uh oh!

vtjnash commented Aug 19, 2025

Uh oh!

JeffBezanson commented Aug 20, 2025

Uh oh!

Uh oh!

NHDaly commented Aug 21, 2025

Uh oh!

NHDaly commented Aug 21, 2025

Uh oh!

JeffBezanson commented Aug 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants