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

Allow named arguments anywhere #491

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

TimWhiting
Copy link
Collaborator

@TimWhiting TimWhiting commented Apr 10, 2024

The main motivation for this feature is to allow trailing lambdas to come after named arguments. I've run into this a lot.

f(1, x="something") fn()
   "something else"

(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 the syntax directory. I moved those tests into a syntax/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.

div(
  classes="x,y,z", 
  div(
...
))

Could be written like this, but becomes noisy and causes deeper indentation (depends how you format it)

div(
  classes="x,y,z", 
  child=div(
...
))

@TimWhiting TimWhiting marked this pull request as ready for review April 10, 2024 19:39
@TimWhiting TimWhiting force-pushed the named-anywhere branch 2 times, most recently from c614214 to 593bb1a Compare April 10, 2024 19:42
Copy link
Collaborator Author

@TimWhiting TimWhiting Apr 10, 2024

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).

Copy link
Collaborator Author

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
Copy link
Collaborator Author

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")
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

Copy link
Collaborator Author

@TimWhiting TimWhiting May 3, 2024

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.

@TimWhiting TimWhiting added this to the Small Release milestone Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant