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

next accepts named arguments #6394

Open
straight-shoota opened this issue Jul 16, 2018 · 10 comments
Open

next accepts named arguments #6394

straight-shoota opened this issue Jul 16, 2018 · 10 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser topic:compiler

Comments

@straight-shoota
Copy link
Member

Arguments to next are parsed like arguments to method calls, but they shouldn't support named parameters:

next 1, foo: 2

This should probably raise a syntax error. Could also capture the named arguments and provide them somehow to the yielding scope, but that's probably too complicated - and not really useful.

If only named arguments are provided next foo: 2, it raises Index out of bounds.

@jhass jhass added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler topic:compiler:parser labels Jul 16, 2018
@wooster0
Copy link
Contributor

wooster0 commented Jul 16, 2018

That's also happening at break.

@makenowjust
Copy link
Contributor

return statement accepts named arguments (of course not work). Because they are parsed by same parse_control_expression method.

Additionally yield accepts named arguments also, but we cannot refer these values. It should be fixed.

@straight-shoota
Copy link
Member Author

straight-shoota commented Jul 17, 2018

If no positional arguments are present, named arguments could perhaps be transformed to a named tuple. So next foo: bar would be equal to next {foo: bar}. But it's not a huge difference, so probably not worth it to have this special case.

@asterite
Copy link
Member

All of this works in Ruby, I would do nothing regarding this for now.

@makenowjust
Copy link
Contributor

foo = while true
        break 1, foo: 1
      end
p foo

In Ruby it yields [1, {foo: 1}], but in Crystal it yields what?

@makenowjust
Copy link
Contributor

Wow! Crystal ignore break and next value inside while loop... #1277

@oprypin
Copy link
Member

oprypin commented Apr 13, 2021

I just checked and #10566 did not resolve this one.

@HertzDevil
Copy link
Contributor

#10566 is only concerned with how while expressions interpret break arguments; it doesn't change what is supported by the break expressions themselves (and by extension the remaining control statements).

Ruby's behaviour here is to combine all named arguments, including from double splats (analogous to #10193), into a hash as the last value. But then there is the following oddity:

def foo(**opts)
  yield **opts
end

foo { |x| x }  # => {}
foo { |*x| x } # => []

In Ruby 3 the first line will return nil. By the way, only yield in Ruby allows named arguments alone (yield x: 1); the other control statements require at least one positional argument if named ones are also present, otherwise it is a syntax error.

I am not sure which behaviour we would choose, or even whether we need them. IMO it shouldn't be possible to mix positional arguments and named arguments in the same parameter:

def foo
  yield 1, 2
  yield 3, a: 4
end

foo { |_, y| typeof(y) } # => (Int32 | NamedTuple(a: Int32))

@straight-shoota
Copy link
Member Author

I would just make named arguments from these keywords an error and be done with it. I don't see much usefulness for using named arguments with next, break or yield. If you want to pass a named tuple, you can just use a named tuple literal.

@HertzDevil
Copy link
Contributor

For break, next, and return I agree. For yield there is actually the possibility of supporting named parameters in blocks. In Ruby:

def foo
  yield a: 1, b: 2
end

foo do |b:, **ns|
  [b, ns] # => [2, {:a=>1}]
end

def bar
  yield 1, 2, 3, 4, a: 5, b: 6, c: 7, d: 8
end

bar do |x0, x1, *xs, b:, a:, **ns|
  [x0, x1, xs, b, a, ns] # => [1, 2, [3, 4], 6, 5, {:c=>7, :d=>8}]
end

A Crystal analogue would be:

def foo
  yield a: 1, b: 2
end

foo do |*, b, **ns|
  {b, ns} # => {2, {a: 1}}
end

def bar
  yield 1, 2, 3, 4, a: 5, b: 6, c: 7, d: 8
end

bar do |x0, x1, *xs, b, a, **ns|
  {x0, x1, xs, b, a, ns} # => {1, 2, {3, 4}, 6, 5, {c: 7, d: 8}}
end

However this is very tricky to implement because it implies all Procs must also support named parameters. (Ruby lambdas do support them.) So perhaps disallowing named arguments in all control statements is the only sensible thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:parser topic:compiler
Projects
None yet
Development

No branches or pull requests

7 participants