julia icon indicating copy to clipboard operation
julia copied to clipboard

WIP: fix #52385 by supporting Union{} inside Tuple{}

Open JeffBezanson opened this issue 1 year ago • 4 comments

This fixes the helpful example from https://github.com/JuliaLang/julia/issues/52385#issuecomment-1969949448. More examples would be great.

This changes the "core" intersection algorithm to return e.g. Tuple{Union{}} for disjoint tuple elements; however there are wrappers around it that still give Union{} since that is the result wanted for most use cases, e.g. argument types and types of values generally. The subtype.jl tests call the "core" algorithm as _type_intersect which I have disabled for now. It will need separate tests.

fix #52385

JeffBezanson avatar Jun 13 '24 21:06 JeffBezanson

Note most of the changes in 35e4a1f9689f4b98f301884e0683e4f07db7514b are generally good, so it doesn't need to be reverted.

JeffBezanson avatar Jun 13 '24 21:06 JeffBezanson

I think this is the way we want to go, but messing with type intersection is pretty risky so I don't think we can merge this into 1.11 this late in the cycle.

JeffBezanson avatar Jun 18 '24 18:06 JeffBezanson

The optimization in #49393 needs to be reverted, as the TODO comment from before that PR is not a legal outcome if this PR is merged.

The usages of the predicates for isconcretetype, isleaftype, isdispatchtuple, and the maybe_subtype_of_cache computations need to be looked at for correctness, as they no longer can be used to refine inference / typemap / subtyping results for Tuple (via the assumption that a leaf-type result implies there is no subtypes of that to examine). It doesn't look like we fully took advantage of the error existence yet for some of those, so some of them may not require many tweaks.

The methods lookup code also may need some work, as it may presuppose that the above restriction was true of all inputs. In particular, code may have assumed that S <: T implies that methods(S) ⊆ methods(T), which I think should imply that there needs to be methods(S) == ∅ if S is a Tuple with a Union{} element. This does not agree with the normal subtyping/intersection definition of method table lookup for S, so it may need to be a special case of some sort.

Could you supply some numbers, once those are fixed, how much this affects the size and performance of OmniPackage and system images?

vtjnash avatar Jun 23 '24 17:06 vtjnash

should imply that there needs to be methods(S) == ∅ if S is a Tuple with a Union{} element

That seems totally fine to me; in the context of methods we know an argument being Union{} is never actually possible so there ought to be many places where we can prohibit and don't need to handle such types.

JeffBezanson avatar Jun 26 '24 18:06 JeffBezanson