Project

General

Profile

Bug #15620

Block argument usage affects lambda semantic

Added by alanwu (Alan Wu) over 1 year ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.7.0dev (2019-11-12T17:29:13Z master a5448c46bd) [x86_64-darwin18]
[ruby-core:91616]

Description

The following snippet demonstrate the issue:

def pass_after_use(&block)
  raise unless block
  lambda(&block).call
end

def direct_pass(&block)
  lambda(&block).call
end

pass_after_use do |_arg|
  puts "fine, because block is materialized into a Proc before it is passed to #lambda"
end

direct_pass do |_arg|
  puts "Raises because all args are required. This is not printed"
end

Output:

fine, because block is materialized into a Proc before it is passed to #lambda
Traceback (most recent call last):
    2: from lambda-block-pass.rb:14:in `<main>'
    1: from lambda-block-pass.rb:7:in `direct_pass'
lambda-block-pass.rb:14:in `block in <main>': wrong number of arguments (given 0, expected 1) (ArgumentError)

I think having the line raise unless block affect Kenrel#lambda's semantic is pretty surprising. Note that if I do raise unless block_given?, call to the lambda without arg also raises.

If I was to decide, I would always have the resulting lambda have required arguments even after multiple levels of block pass. That is, as long as the original block is a literal block.

This is either a breaking change or a regression from 2.4. The same script executes without raising in 2.4.5 (block arguments are always materialized).


Related issues

Related to Ruby master - Feature #15973: Let Kernel#lambda always return a lambdaClosedmatz (Yukihiro Matsumoto)Actions

Updated by marcandre (Marc-Andre Lafortune) over 1 year ago

  • Assignee set to matz (Yukihiro Matsumoto)

Current behavior is clearly a bug due to the block passing implementation.

It's actually not clear to me why lambda(&proc{}).lambda? returns false. It doesn't seem useful and is counter-intuitive to me.

If others think we could revisit this, it has been clearly documented as such for over 10 years (r14713), so changing this behavior would be a breaking change. I would guess with very little impact. A quick search in the top 500 gems revealed a single use of lambda(&...) which doesn't look incompatible: https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/resolver.rb#L435

I found two other uses in specs, rubocop: spec/rubocop/cop/style/stabby_lambda_parentheses_spec.rb and in vcr: spec/lib/vcr/structs_spec.rb. Neither seem problematic.

We could deprecate this use with warning and then change it?

Otherwise we could simply fix the regression.

Updated by Eregon (Benoit Daloze) over 1 year ago

lambda(&proc{}).lambda? returns false because I think the rule is:
once a Proc is created, it never changes its lambda-ness.

So the only way to create a lambda is passing a block directly to lambda (or through send), or using ->.

So I think direct_pass above should create a non-lambda Proc, and I would argue it's a bug it creates a lambda since MRI 2.5.

Updated by marcandre (Marc-Andre Lafortune) over 1 year ago

Eregon (Benoit Daloze) wrote:

lambda(&proc{}).lambda? returns false because I think the rule is:
once a Proc is created, it never changes its lambda-ness.

Right. I was wondering why this is the "rule", what's the rationale. It makes lambda(&...) equivalent to ...to_proc but less clear. I was thinking only in terms of parameter passing (in which case the rule feels counter productive), but there's also handling of break and return (in which case the rule makes more sense I guess, still not convinced).

Note that the rule can be broken, for example with:

def transform_proc_into_lambda(&proc)
  o = Object.new
  o.singleton_class.define_method(:foo, &proc)
  o.method(:foo).to_proc
end

transform_proc_into_lambda{}.lambda? # => true

Updated by Eregon (Benoit Daloze) over 1 year ago

Right, define_method is kind of an exception.
However, it doesn't mutate the lambda-ness of the Proc, it creates a new Proc from that block with lambda semantics.

It's still confusing for the user though, as a given literal block could be used as both Proc and lambda, which is likely unexpected (e.g. break/return cannot work correctly in that block).
This can also be achieved with send(:proc or :lambda) { ... }, but that case is more explicit about it.

Updated by alanwu (Alan Wu) over 1 year ago

Since normal Ruby methods can't differentiate between a literal block and a block pass, having #lambda behave like a normal method gives us more consistency.
#lambda doesn't need to mutate its argument, it could return a lambda proc based on the block-passed proc.

Updated by ko1 (Koichi Sasada) over 1 year ago

we will fix it.

#7

Updated by Hanmac (Hans Mackowiak) over 1 year ago

nobu (Nobuyoshi Nakada) should this one be closed too?

Updated by Eregon (Benoit Daloze) about 1 year ago

I propose we fix the regression first, the optimization introduced let's fix that, regardless of desired semantics in the future.

Updated by Eregon (Benoit Daloze) about 1 year ago

I posted more thoughts on #15973. Maybe we should simply raise when not giving a literal block to lambda/proc, since it's useless or dangerous (changes semantics of code) otherwise.

Updated by alanwu (Alan Wu) about 1 year ago

Here is a patch which restores the 2.4 behavior:

https://github.com/ruby/ruby/pull/2289

Passes make check with VM_CHECK_MODE=1.

Updated by ko1 (Koichi Sasada) about 1 year ago

  • Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)
  • Status changed from Open to Assigned
#12

Updated by alanwu (Alan Wu) 9 months ago

  • ruby -v set to ruby 2.7.0dev (2019-11-12T17:29:13Z master a5448c46bd) [x86_64-darwin18]
#13

Updated by Eregon (Benoit Daloze) 8 months ago

  • Related to Feature #15973: Let Kernel#lambda always return a lambda added
#14

Updated by alanwu (Alan Wu) 8 months ago

  • Status changed from Assigned to Closed

Applied in changeset git|85a337f986fe6da99c7f8358f790f17b122b3903.


Kernel#lambda: return forwarded block as non-lambda proc

Before this commit, Kernel#lambda can't tell the difference between a
directly passed literal block and one passed with an ampersand.

A block passed with an ampersand is semantically speaking already a
non-lambda proc. When Kernel#lambda receives a non-lambda proc, it
should simply return it.

Implementation wise, when the VM calls a method with a literal block, it
places the code for the block on the calling control frame and passes a
pointer (block handler) to the callee. Before this commit, the VM
forwards block arguments by simply forwarding the block handler, which
leaves the slot for block code unused when a control frame forwards its
block argument. I use the vacant space to indicate that a frame has
forwarded its block argument and inspect that in Kernel#lambda to detect
forwarded blocks.

This is a very ad-hoc solution and relies heavily on the way block
passing works in the VM. However, it's the most self-contained solution
I have.

[Bug #15620]

Also available in: Atom PDF