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

[GR-18163] Fix declaring a variable inside the case target expression #3378

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Bug fixes:
* Fix some C API functions which were failing when called with Ruby values represented as Java primitives (#3352, @eregon).
* Fix `IO.select([io], nil, [io])` on macOS, it was hanging due to a bug in macOS `poll(2)` (#3346, @eregon, @andrykonchin).
* Run context cleanup such as showing the output of tools when `SignalException` and `Interrupt` escape (@eregon).
* Handle a new variable inside the `case` target expression correctly (#3377, @eregon).

Compatibility:

Expand Down
16 changes: 16 additions & 0 deletions spec/ruby/language/case_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,22 @@ def ===(o)
:called
end.should == :called
end

it "supports declaring variables in the case target expression" do
def test(v)
case new_variable_in_expression = v
when true
# This extra block is a test that `new_variable_in_expression` is declared outside of it and not inside
self.then { new_variable_in_expression }
else
# Same
self.then { new_variable_in_expression.casecmp?("foo") }
end
end

self.test("bar").should == false
self.test(true).should == true
end
end

describe "The 'case'-construct with no target expression" do
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/truffleruby/parser/BodyTranslator.java
Original file line number Diff line number Diff line change
Expand Up @@ -693,8 +693,6 @@ protected ArgumentsAndBlockTranslation translateArgumentsAndBlock(SourceIndexLen
public RubyNode visitCaseNode(CaseParseNode node) {
final SourceIndexLength sourceSection = node.getPosition();

RubyNode elseNode = translateNodeOrNil(sourceSection, node.getElseNode());

/* There are two sorts of case - one compares a list of expressions against a value, the other just checks a
* list of expressions for truth. */

Expand All @@ -710,6 +708,8 @@ public RubyNode visitCaseNode(CaseParseNode node) {
/* Build an if expression from the whens and else. Work backwards because the first if contains all the
* others in its else clause. */

RubyNode elseNode = translateNodeOrNil(sourceSection, node.getElseNode());

for (int n = node.getCases().size() - 1; n >= 0; n--) {
final WhenParseNode when = (WhenParseNode) node.getCases().get(n);

Expand Down Expand Up @@ -746,6 +746,8 @@ public RubyNode visitCaseNode(CaseParseNode node) {
// A top-level block assigns the temp then runs the if
ret = sequence(sourceSection, Arrays.asList(assignTemp, ifNode));
} else {
RubyNode elseNode = translateNodeOrNil(sourceSection, node.getElseNode());

for (int n = node.getCases().size() - 1; n >= 0; n--) {
final WhenParseNode when = (WhenParseNode) node.getCases().get(n);

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/truffleruby/parser/YARPTranslator.java
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,6 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
// - the other just checks a list of expressions for truth.

final RubyNode rubyNode;
RubyNode elseNode = translateNodeOrNil(node.consequent);

if (node.predicate != null) {
// Evaluate the case expression and store it in a local
Expand All @@ -875,6 +874,8 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {

// Build an if expression from `when` and `else` branches.
// Work backwards to make the first if contain all the others in its `else` clause.
RubyNode elseNode = translateNodeOrNil(node.consequent);

final Nodes.Node[] conditions = node.conditions;

for (int n = conditions.length - 1; n >= 0; n--) {
Expand Down Expand Up @@ -936,6 +937,8 @@ public RubyNode visitCaseNode(Nodes.CaseNode node) {
// Build an if expression from `when` and `else` branches.
// Work backwards to make the first if contain all the others in its `else` clause.

RubyNode elseNode = translateNodeOrNil(node.consequent);

final Nodes.Node[] conditions = node.conditions;

for (int n = node.conditions.length - 1; n >= 0; n--) {
Expand Down