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

Continue with anonymous block arguments #8764

Open
straight-shoota opened this issue Feb 8, 2020 · 4 comments
Open

Continue with anonymous block arguments #8764

straight-shoota opened this issue Feb 8, 2020 · 4 comments

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Feb 8, 2020

Follows up on #8117. This PR added syntax for anonymous block arguments def foo(&). The idea is to use this syntax exclusively for yielding methods.
Currently it's valid to completely omit the block argument when there's yield in the body. This hides the yielding nature of the method from the signature and API docs. Such an essential property should be made visible.

After the initial syntax was added, we switched usage in stdlib in #8394.

The next step would be to deprecate some of the currently valid method signatures. The idea as proposed in #8117:

  • Just & means a non-captured block argument: because it doesn't have a name there's no way to capture it
  • &block means a captured block argument, and the compiler will verify that it's used in the semantic phase (because it might be mentioned inside a macro)
  • &block without a mention will give a compile error suggesting to either change it to & to avoid a closure or to mention it somehow
  • if you yield and the method signature doesn't have & the compiler will tell you

This would be a breaking change for yielding methods with a named block argument or no block argument at all, and also for methods with a named block argument that's never used in the literal scope of its body. But it improves clarity and helps to better identify the behaviour of block arguments.

Part of #7157

@RX14
Copy link
Contributor

RX14 commented Feb 9, 2020

This hides the yielding nature of the method from the signature and API docs.

No, it shows up in the API docs if you use yield. We shouldn't force people to annotate their method with an unneccesary & if they use yield but don't want to annotate the arg types. The rest of the changes are great.

@straight-shoota
Copy link
Member Author

straight-shoota commented Feb 9, 2020

Oh my bad. The API docs already represent this correctly.

I'd still prefer always specifying & on yielding methods (types can be omitted obviously). Even if it's technically unnecessary, it still improves readability of the source code. We already put & in the signature in the API docs, but reading code is almost as important.
I acknowledge that this doesn't need to be enforced by the compiler, though. It could just be a style guide. Maybe consider it for the formatter?

@HertzDevil
Copy link
Contributor

HertzDevil commented Jan 8, 2022

It seems Ruby eventually wants to require & in methods that accept a block too.

For context, Ruby 3.1 allows a bare & to forward the anonymous block argument, whether it is captured or not (see #12110). Block forwarding requires a corresponding bare & parameter:

def foo1(&)
  yield
end

def bar1(&)
  foo1(&)
end

bar1 { 1 } # => 1

def foo2(&block)
  block.call
end

def bar2(&)
  proc(&) # captures anonymous block
  foo2(&)
end

bar2 { 1 } # => 1

I don't know whether anonymous block forwarding is exactly equivalent to a named one which would capture the block. It is definitely not a syntactic transformation though.

Also of note is that Def#block_arg returns a Nop unless the block parameter is used in the method body: (#5334)

module Foo
  def a1(&);                       end
  def b1(&block);                  end
  def c1(& : ->);                  end
  def d1(&block : ->);             end
  def a2(&);           yield;      end
  def b2(&block);      yield;      end
  def c2(& : ->);      yield;      end
  def d2(&block : ->); yield;      end
  def b3(&block);      block.call; end
  def d3(&block : ->); block.call; end
end

{% Foo.methods.map(&.block_arg) %}                           # => [, , , , , , , , block, block : (->)]
{% Foo.methods.select(&.block_arg.is_a?(Arg)).map(&.name) %} # => [b3, d3]

Likewise, when the Def nodes are printed only b3 and d3 retain their block parameters. I think this should change too, even if it means some of the Args will have an empty name.

@zw963
Copy link
Contributor

zw963 commented Dec 11, 2022

No, it shows up in the API docs if you use yield. We shouldn't force people to annotate their method with an unneccesary & if they use yield but don't want to annotate the arg types. The rest of the changes are great.

i don't agree with this idea, if yield exists in method body, block must be provided when invoke this method, add a extra & in method parameter is more clear way reveal this.

Also see following code for a really use case need to improve in std-lib.

def accept
sock = accept
begin
yield sock
ensure
sock.close
end
end

See discuss here, A extra & in the pamameter will make issue like this more clearly, user don't need search yield in (probalby a big) method body too find out whether this method need a block.

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

No branches or pull requests

4 participants