Project

General

Profile

Bug #16181

return from a proc in a module/class body returns out of script. Should be LJE.

Added by enebo (Thomas Enebo) 9 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:95090]

Description

return is not allowed from class/module body. But if we insert a return into a block then we can invoke the block then it returns all the way out of the script. This has to be accidental behavior doesn't it? I believe the case below should end up as a LocalJumpError:

class Foo
  proc { return }.call
end
puts "NEVER SEEN"

This behavior started some time in 2.5 (it used to be an LJE).

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

It looked into this behavior, which started in Ruby 2.4 (when top level return started being allowed).

return not being allowed directly in class/module body is a parse error, because the parser knows that it cannot be valid. However, you cannot have the behavior inside of a block inside of class/module be a parse error, because you do not know how the block will be evaluated:

class Foo
  alias proc lambda
  proc { return }.call
end
puts "NEVER SEEN"

Similarly, though this operates as a top level return, the warning for top level return with argument does not take effect, because the compiler doesn't know it will operate as a top level return:

class Foo
  proc { return 1 }.call # no top level return with argument warning
end
puts "NEVER SEEN"

I don't think the compiler has enough information to handle this correctly, because it doesn't know at compilation time whether the block will be executed as a proc or as a lambda.

I do agree that a LocalJumpError is the most appropriate way to handle this.

Updated by enebo (Thomas Enebo) 9 months ago

I agree this is impossible to be done by parser or at iseq generation time. It must be done when return is invoked.

JRuby will return LocalJumpError for this for 9.2.9.0. It also had from 9.2.6.0 and earlier.

As a runtime LJE we know lexically it is defined within a module/class and we can also look up stack to see if it has migrated or not (I assume MRI has even more flexibility in this than JRuby does). Similarly we know it is a lambda or not at that point so that is not really a problem at runtime.

Jeremy, since you are only person who has looked would you say semantically this should be some error vs silently only executing part of a file?

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

enebo (Thomas Enebo) wrote:

Jeremy, since you are only person who has looked would you say semantically this should be some error vs silently only executing part of a file?

I believe if return directly inside class/module is an error, return inside proc inside class/module should also be an error. Since it cannot be a compile time error, it should be a runtime error (LocalJumpError).

Alternatively, we start to allow return inside class/module, and then the current behavior for return inside proc inside class/module makes sense.

Updated by enebo (Thomas Enebo) 9 months ago

Ok cool. Let's hope we get some more consensus on this. I don't like deviating from MRI in behavior so I hope to see more come to the same conclusion.

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

I've added a pull request that makes return in proc in class/module a LocalJumpError: https://github.com/ruby/ruby/pull/2511

Updated by enebo (Thomas Enebo) 9 months ago

I added a comment about removing the pre 2.7 spec as it is unintended behavior.

Updated by jeremyevans0 (Jeremy Evans) 9 months ago

  • Status changed from Open to Closed

Also available in: Atom PDF