Skip to content

Conversation

@jeremyevans
Copy link
Contributor

return directly in class/module is an error, so return in
proc in class/module should also be an error. I believe the
previous behavior was an unintentional oversight during the
addition of top-level return in 2.4.

return directly in class/module is an error, so return in
proc in class/module should also be an error.  I believe the
previous behavior was an unintentional oversight during the
addition of top-level return in 2.4.
@enebo
Copy link
Contributor

enebo commented Oct 1, 2019

@jeremyevans I was responsible for that current spec but I really think we should not leave in that specified behavior (for 2.4-2.6). Your new spec for 2.7 on seems like semantic behavior and good to be in ruby/spec. My original test just happens to verify something which the implementation happens to do.

My recommendation is to remove the pre 2.7 spec because it specs what I consider to be an unintended behavior.

@eregon
Copy link
Member

eregon commented Oct 1, 2019

I'm neutral on removing the spec for Ruby < 2.7, both are fine for me.

@enebo
Copy link
Contributor

enebo commented Oct 1, 2019

I am fine either way as well other than having an opinion. We will just tag it out if it stays (until JRuby 9.3.x which will be 2.7.x). I just don't like seeing non-semantic behavior in ruby/spec (and I am to blame for adding it originally :) ).

@eregon
Copy link
Member

eregon commented Oct 1, 2019

Let's remove the <2.7 spec then since it seems clearly unintended behavior.

This becomes unspecified behavior on earlier Ruby versions.
@jeremyevans jeremyevans merged commit ef69738 into ruby:master Oct 2, 2019
@jeremyevans jeremyevans deleted the return-in-proc-in-class-lje-16181 branch October 2, 2019 14:56
@ko1
Copy link
Contributor

ko1 commented Oct 2, 2019

  • it was merged, but semantic change. don't you need Matz's check?
  • it seems we can check it at compile time.

@ko1
Copy link
Contributor

ko1 commented Oct 2, 2019

... i agree it is unexpected behavior, too.

@jeremyevans
Copy link
Contributor Author

I'm sorry. I thought it was a bug, since we do not allow return directly in class/module. I don't think we can check it at compile time:

class Foo
  class << self
    alias proc lambda if ARGV.first == 'lambda'
  end
  proc { return }.call
end

I can definitely revert and wait for matz's decision if you think it is best.

@ko1
Copy link
Contributor

ko1 commented Oct 3, 2019

I agree, too. Wow!

@enebo
Copy link
Contributor

enebo commented Oct 3, 2019

@ko1 do you still think Matz needs to check this? It feels like this is just accidental behavior and not intended.

@ko1
Copy link
Contributor

ko1 commented Oct 3, 2019

Now I don't think we need to ask Matz. I'll ask Matz when I meet him if I remember :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants