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

support optional params with last-position block argument(s) #405

Closed
timotheecour opened this issue Jul 31, 2021 · 6 comments · Fixed by nim-lang/Nim#18631
Closed

support optional params with last-position block argument(s) #405

timotheecour opened this issue Jul 31, 2021 · 6 comments · Fixed by nim-lang/Nim#18631

Comments

@timotheecour
Copy link
Member

timotheecour commented Jul 31, 2021

proposal

support optional params with last-position block argument.
This is a common use case; fixing this would simplify a lot of code.

This shouldn't break any code, but would make new code compile instead of give CT error.

when true: # gitissue
  template fn(a = 1, b = 2, body) = discard
  fn(1, 2): # works
    foo

  # these ccurrently give CT error; under RFC these would work:
  fn(a = 1):
    foo
    bar

  fn(1): foo
  fn(b = 2): foo
  fn(b = 2, a = 1): foo
  fn(a = 1): foo
  fn(): foo
  fn: foo

benefits

this should be obvious; the current workaround is bad, involving writing multiple overloads without optional params, which complicates things (less DRY, complicates documentation, turns the symbol into a symchoice even if it wasn't in the 1st place, which affects binding ...)

another problem with this workaround is that it runs into nim-lang/Nim#17164:

when true:
  template fn(a: int, body: untyped) = discard
  template fn(body: untyped) = discard
  fn:
    foo

which gives: Error: undeclared identifier: 'foo'

notes

As the "do" can only fill the arguments starting from the last one, there is no ambiguity in letting the optional arguments before it be left unfiled.

implementation

  • in sigmatch, constrain last position block argument to match in the last formal position if it's a non-optional param
    the it's a non-optional param condition is so that this continues to work:
when true:
  template fn(a: int, body: untyped, b = 2) = discard
  fn(1): foo
  • if M do params follow a last position block param, constrain those M+1 arguments to match to the last M+1 formal params, starting from the end, while those formal params are non-optional

links

@c-blake
Copy link

c-blake commented Aug 3, 2021

I believe all on its own "named do" might also allow solving almost all these problems by "forcing/allowing" just naming every parameter at a call site in any order, block parameter or not.

Non-block params after block params might look a little weird, requiring myLateParam.do: text to name them, but this is also at the discretion of the naming-everything caller.

That said, I suspect "named do" can probably also coexist with this feature.

@timotheecour
Copy link
Member Author

same comment as #402 (comment), please write a separate RFC, there's not enough details in that link to assess your proposal.

@timotheecour timotheecour changed the title support optional params with last-position block argument support optional params with last-position block argument(s) Aug 3, 2021
@Varriount
Copy link

Although no specifics have been mentioned yet regarding "named do", I do agree with the general idea. Having different signature matching behavior for this class of notation is... odd.

@c-blake
Copy link

c-blake commented Aug 5, 2021

I agree it could use its own RFC. (@Varriount - maybe you can do that? Someone else? Not sure when I will get to that.)

The "named do" idea is simple enough that there are only two real choices - what it looks like and what the AST looks like. E.g. just name.do: dot call or also do(name): or name=do:. The produced AST could be identical to what regular named parameters produce (as strictly "alternate syntax") for macro backward compatibility or be its own new AST wrinkle.

Other than those choices, named-do is just a natural way to extend the usual named parameter passing notation to do-blocks which makes call sites both order-flexible and more descriptive, just like regular named parameters. Just like ordinary named params, because of how permutation scales you, need 3..4 to really pay off, and 2 to even matter (but even with 2 you will get divergence of call-site-author-choice like the init/edit vs edit/init in the Forum post).

If it were in place, I think the need for this semantics/sigmatch work is softened quite a bit, as mentioned. I say it again only by way of explaining why I commented here. The two might also just peacefully coexist. To me it seemed worth thinking through in the context of block-passing calling notation.

@Araq
Copy link
Member

Araq commented Aug 11, 2021

As I've said it on a related RFC, this is "lipstick on a pig" and I consider the unification of typed/untyped parameters to be the proper solution. There is also nim-lang/Nim#14863 which is yet another "bugfix" of the same sort. And that's on top of the "shadow scope" implementation which is also a workaround for the typed/untyped behavior and is not yet spec'ed out either.

I don't want to let the perfect be the enemy of the good but at some point we have to consider that these bugfixes and enhancements further cement a design flaw and take up resources that could be spent on solving the core issue. I claim that the core issue is that we have no spec for typed ASTs and yet typed ASTs are the only proper solution that enables DSLs that can compose.

@timotheecour
Copy link
Member Author

timotheecour commented Aug 11, 2021

As I've said it on a related RFC, this is "lipstick on a pig" and I consider the unification of typed/untyped parameters to be the proper solution.

unifying typed/untyped is orthogonal to this RFC; last position typed params would benefit from ability to have optional params and all the points raised in this RFC would apply even if typed/untyped were unified, except for the one relevant to nim-lang/Nim#17164.

In any case, unifying typed/untyped should be discussed in a dedicated RFC instead of scattering discussion in many places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants