-
Notifications
You must be signed in to change notification settings - Fork 167
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
Allow named arguments anywhere #491
base: dev
Are you sure you want to change the base?
Conversation
d80a601
to
b76d3aa
Compare
c614214
to
593bb1a
Compare
test/syntax/run/tail.kk.out
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a bug (that is currently also on dev, where implicit resolution considers but doesn't accept functions which would match using a default for an optional parameters).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #493
= do let (nfixed,rest) = span (isNothing . fst) nargs | ||
(nnamed,morefixed) = span (not . isNothing . fst) rest | ||
= do let nfixed = filter (isNothing . fst) nargs | ||
nnamed = filter (not . isNothing . fst) nargs | ||
fixed = map snd nfixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using span, we just filter by whether they have a name.
fixed = map snd nfixed | ||
named = [((name,rng),expr) | (Just (name,rng),expr) <- nnamed] | ||
-- check that named arguments all come after the positional ones | ||
case morefixed of | ||
[] -> return () | ||
(arg:_) -> infError (getRange (snd arg)) (text "positional arguments cannot follow named arguments") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No error is needed any more!
@@ -138,10 +138,11 @@ matchArguments matchSome range free tp fixed named mbExpResTp | |||
then unifyError NoMatch | |||
else do -- trace (" matchArguments: " ++ show (map pretty pars, map pretty fixed, map pretty named)) $ return () | |||
-- subsume fixed parameters | |||
let (fpars,npars) = splitAt (length fixed) pars | |||
let parsNotNamedArg = filter (\(nm,tp) -> nm `notElem` map fst named) pars | |||
let (fpars,rest) = splitAt (length fixed) parsNotNamedArg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other change really is during unification removing all the supplied named arguments from parameters before determining which ones are fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than two more one line changes just below this, all of the rest of the changes are in updating tests.
593bb1a
to
737a1c9
Compare
737a1c9
to
7ce5203
Compare
The main motivation for this feature is to allow trailing lambdas to come after named arguments. I've run into this a lot.
(Currently an error).
Essentially, instead of thinking of formal parameters as named or fixed, we consider them required or optional, with implicit parameters being required parameters, but optional as arguments.
Arguments are still thought of as named or fixed.
Subsumption proceeds as follows:
After matching the named arguments to formal parameters, the fixed arguments must match the ordering of the remaining parameters, with optional parameters staying optional.
In making these changes I noticed that the
syntax/run
test subdirectory was not actually being run, since it inherited a-l
flag from thesyntax
directory. I moved those tests into asyntax/no-run
subdirectory to prevent the conflicting flags.Alternatively we could adjust the trailing lambda desugaring to put it prior to all named arguments. See PR #533
The main advantage of this particular solution is that it also allows for function "configuration" to happen prior to giving common arguments that would cause more indentation and visual clutter / noise if named.
Could be written like this, but becomes noisy and causes deeper indentation (depends how you format it)