Avoid giving an under-application error for 0 arguments #6042
Conversation
This seems like an erroneous case that wasn't noticed, but can occur in some situations, like an existential vs. universal mismatch. If at least one argument type wasn't peeled, then it doesn't make sense to suggest that the function is under-applied. So just guard against it.
| (_, Just (Right {})) -> pure $ Mismatch ft et fl el mismatchSite n (Just SuperfluousDelay) | ||
| (Just needArgs, _) -> pure $ FunctionUnderApplied ft et fl el mismatchSite n (lowerType <$> needArgs) | ||
| (Just needArgs, _) | ||
| | not (null needArgs) -> pure $ FunctionUnderApplied ft et fl el mismatchSite n (lowerType <$> needArgs) |
There was a problem hiding this comment.
I'm way out of my element but I'm curious: is there anything that should be preventing mayNeedArgs from being Just for this case, instead checking here whether needArgs is empty? Is the place that is producing that Just misdiagnosing the error?
There was a problem hiding this comment.
I think that a Nothing means that no amount of argument dropping allows for a subtyping relationship to exist. So, it indicates a complete mismatch, like Int vs String or something.
I'm not quite certain how the subtyping check succeeds in the test when it apparently failed in the type checker. Maybe some context is lost by the time it is in the error parser. I didn't actually write this case or study it very closely.
There was a problem hiding this comment.
A quick glance at findUnderApplication implies it should probably return Maybe (NonEmpty (Type v a)) or w/e; looks like it's unifying the type var with the last arg and is falsely reporting an under-application;
What you've written here is equivalent behaviour since it's only used in this one spot; but findUnderApplication should probably just should have a top-level guard which asserts there are actually args in it then return the NonEmpty
ChrisPenner
left a comment
There was a problem hiding this comment.
Would likely be preferable to change findUnderApplication itself so we don't accidentally copy-pasta this error somewhere else, but the end result is the same 👍🏼
Thanks for handling this one!
|
Not sure whether to merge this yet or @dolio did you want to do this?
|
|
I'm uncertain of the exact suggestion there. |
This seems like an erroneous case that wasn't noticed, but can occur in some situations, like an existential vs. universal mismatch. If at least one argument type wasn't peeled, then it doesn't make sense to suggest that the function is under-applied. So just guard against it.
from https://unisoncomputing.slack.com/archives/CLGTK464E/p1764610297238729