Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add morespecific rule for Type{Union{}} #49349

Merged
merged 4 commits into from
Apr 20, 2023
Merged

add morespecific rule for Type{Union{}} #49349

merged 4 commits into from
Apr 20, 2023

Commits on Apr 18, 2023

  1. update Statistics.jl 

    Removes some overly strict `@test_throws MethodError` for calls with
`Union{}` or `Any`, in preparation for making those errors more precise.
    @vtjnash
    vtjnash committed Apr 18, 2023
    Configuration menu
    Copy the full SHA
    dccef3b View commit details
    Browse the repository at this point in the history

  2. morespecific: add rule for Type{Union{}} 

    Make Type{Union{}} in method definitions always the most specific type
(normally these would end up being ambiguous). This ensures we do not
invalidate them, nor need to consider ambiguities that might arise from
intersections with them.
    @vtjnash
    vtjnash committed Apr 18, 2023
    Configuration menu
    Copy the full SHA
    6ec0a1a View commit details
    Browse the repository at this point in the history

  3. add typemap filtering option for Union{} 

    Based on the new morespecific rule for Union{} and method definitions of
the specific form `f(..., Type{Union{}}, Vararg)`. If a method
definition exists with that specific form, the intersection visitor will
ignore all intersections that have that as their only result, saving
significant effort when working with lookups involving `Type{<:T}`
(which usually ended up mostly ambiguous anyways).

Fixes: #33780

This pattern turns out to have still to been making package loading
slow. We could keep adding methods following the ambiguity pattern
#46000 for the couple specific
functions that need it (constructor, eltype, IteratorEltype,
IteratorSize, and maybe a couple others) so the internals can detect
those and optimize functions that have that method pair. But it seems
somewhat odd, convoluted, and non-obvious behavior there. Instead, this
breaks all ambiguities in which Union{} is present explicitly in favor
of the method with Union{}. This means that when computing method
matches, as soon as we see one method definition with Union{}, we can
record that the method is the only possible match for that slot.

This, in essence, permits creating a rule for dispatch that a TypeVar
lower bound must be strictly a supertype of Union{}, but this creates it
at the function level, instead of expecting the user to add it to every
TypeVar they use to define methods.

This also lets us improve the error message for these cases (generally
they should error to avoid polluting the inference result), since we can
be assured this method will be called, and not result in an ambiguous
MethodError instead!

Reverts the functional change of #46000
    @vtjnash
    vtjnash committed Apr 18, 2023
    Configuration menu
    Copy the full SHA
    a08e327 View commit details
    Browse the repository at this point in the history

  4. also optimize {Type{T},T} lookup 

    Since T cannot be Union{} here, the prior optimization would not get
detected post facto, but a priori this cannot be inhabited for
T=Union{}, so we can exclude it immediately. This does not happen during
inference, but shows up during edge validation somewhat often.
    @vtjnash
    vtjnash committed Apr 18, 2023
    Configuration menu
    Copy the full SHA
    285c770 View commit details
    Browse the repository at this point in the history