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.

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

[WIP] Improve overload resolution #20054

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Commits on Aug 19, 2024

  1. Improve overload resolution

    `resolveOverloaded1` starts with `narrowMostSpecific(candidates)`
    which compares the alternatives based on the 1st argument list.
    If more than one result if found, we can continue with the 2nd argument list.
    But we do so with the original candidates, rather than with the result of `narrowMostSpecific`.
    This can lead to choosing an alternative which is not the most specific,
    by now disregarding the 1st argument list.
    
    In i120053, the 1st pass correctly eliminated the 3rd `def ^^^`,
    but could not resolve between the 1st two (having the same argument list).
    The 2nd pass then disregarded this and restarted the comparison
    based on the 2nd argument list alone, which incorrectly yielded the 3rd option.
    
    The change is simply using the initial result of `narrowMostSpecific` in the recursive resolution based on subsequent argument lists.
    I'm not sure however if the same changes should apply to the rest of the cases attempting further narrowing ?
    EugeneFlesselle committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    1fdc780 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    04a59ae View commit details
    Browse the repository at this point in the history

Commits on Aug 20, 2024

  1. Make overloading resolution changes apply in 3.7 and report warnings …

    …in 3.6
    
    This is similar to the warnings for changes in given preference.
    In this case, however, the changes only affect a part of disambiguation used
    relatively late in the process, as a "last resort" disambiguation mechanism.
    We can therefore accept running resolution independently with both schemes
    in these cases to detect and report changes.
    
    Having a source position for the warning messages requires passing the tree
    source position to resolveOverloaded from the Typer.
    It could previously be avoided this since any potential error in overloading
    could be determined from its result. Clearly, this cannot be done for the new
    warnings, although I am open to an alternative design.
    EugeneFlesselle committed Aug 20, 2024
    Configuration menu
    Copy the full SHA
    2e0cff0 View commit details
    Browse the repository at this point in the history
  2. Test overloading changes and reported warning messages

    Note that in tests/neg/multiparamlist-overload-3.7 Test2 we now
    get an error in both Parts as intended. But they are, however, different ones:
    Part1 is a TypeMismatch Error, whereas Part2 is a NoMatchingOverload Error.
    
    This is due to the fact that `resolveOverloaded` will first
    find the candidate alternatives by considering only the 1st parameter list
    and commit early if there is a single one, e.g. Test2.Part1.
    If not, we recursively continue with the found alternatives and
    recompute the candidate alternatives based on the 2nd parameter list,
    which may rule out all of them, and hence lead to a different message.
    EugeneFlesselle committed Aug 20, 2024
    Configuration menu
    Copy the full SHA
    f40c609 View commit details
    Browse the repository at this point in the history