NoMatchingPatternError should not re-evaluate the case expression

Author: eregon

spec/ruby/language/pattern_matching_spec.rb

@@ -251,6 +251,18 @@
     }.should raise_error(NoMatchingPatternError, /\[0, 1\]/)
   end
 
+  it "raises NoMatchingPatternError if no pattern matches and evaluates the expression only once" do
+    evals = 0
+    -> {
+      eval <<~RUBY
+        case (evals += 1; [0, 1])
+        in [0]
+        end
+      RUBY
+    }.should raise_error(NoMatchingPatternError, /\[0, 1\]/)
+    evals.should == 1
+  end
+
   it "does not allow calculation or method calls in a pattern" do
     -> {
       eval <<~RUBY

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -837,11 +837,24 @@ public RubyNode visitCaseNode(CaseParseNode node) {
     @Override
     public RubyNode visitCaseInNode(CaseInParseNode node) {
         final SourceIndexLength sourceSection = node.getPosition();
-        RubyNode caseExprValue = node.getCaseNode().accept(this);
+
+        PatternMatchingTranslator translator = new PatternMatchingTranslator(language, source, parserContext,
+                currentNode, node.getCases(), environment, this);
+
+        // Evaluate the case expression and store it in a local
+
+        final RubyNode caseExprValue = node.getCaseNode().accept(this);
+        final int tempSlot = environment.declareLocalTemp("case in value");
+        final ReadLocalNode readTemp = environment.readNode(tempSlot, sourceSection);
+        final RubyNode assignTemp = readTemp.makeWriteNode(caseExprValue);
+
+        /* Build an if expression from the ins and else. Work backwards because the first if contains all the others in
+         * its else clause. */
+
         RubyNode elseNode;
         if (node.getElseNode() == null) {
             RubyCallNodeParameters inspectCallParameters = new RubyCallNodeParameters(
-                    caseExprValue,
+                    NodeUtil.cloneNode(readTemp),
                     "inspect",
                     null,
                     EmptyArgumentsDescriptor.INSTANCE,
@@ -854,20 +867,6 @@ public RubyNode visitCaseInNode(CaseInParseNode node) {
             elseNode = translateNodeOrNil(sourceSection, node.getElseNode());
         }
 
-        PatternMatchingTranslator tr = new PatternMatchingTranslator(language, source, parserContext,
-                currentNode, node.getCaseNode(), node.getCases(), environment, this);
-
-        final RubyNode ret;
-
-        // Evaluate the case expression and store it in a local
-
-        final int tempSlot = environment.declareLocalTemp("case in value");
-        final ReadLocalNode readTemp = environment.readNode(tempSlot, sourceSection);
-        final RubyNode assignTemp = readTemp.makeWriteNode(caseExprValue);
-
-        /* Build an if expression from the ins and else. Work backwards because the first if contains all the others in
-         * its else clause. */
-
         for (int n = node.getCases().size() - 1; n >= 0; n--) {
             final InParseNode in = (InParseNode) node.getCases().get(n);
 
@@ -876,7 +875,7 @@ public RubyNode visitCaseInNode(CaseInParseNode node) {
             // us we-using the 'when' parser for 'in' temporarily.
             final ParseNode patternNode = in.getExpressionNodes();
 
-            final RubyNode conditionNode = tr.translatePatternNode(patternNode, readTemp);
+            final RubyNode conditionNode = translator.translatePatternNode(patternNode, readTemp);
             // Create the if node
             final RubyNode thenNode = translateNodeOrNil(sourceSection, in.getBodyNode());
             final IfElseNode ifNode = new IfElseNode(conditionNode, thenNode, elseNode);
@@ -888,7 +887,7 @@ public RubyNode visitCaseInNode(CaseInParseNode node) {
         final RubyNode ifNode = elseNode;
 
         // A top-level block assigns the temp then runs the if
-        ret = sequence(sourceSection, Arrays.asList(assignTemp, ifNode));
+        final RubyNode ret = sequence(sourceSection, Arrays.asList(assignTemp, ifNode));
 
         return addNewlineIfNeeded(node, ret);
     }

src/main/java/org/truffleruby/parser/PatternMatchingTranslator.java

@@ -57,7 +57,6 @@
 
 public class PatternMatchingTranslator extends BaseTranslator {
 
-    final ParseNode data;
     final ListParseNode cases;
     final TranslatorEnvironment environment;
     final BodyTranslator bodyTranslator;
@@ -69,12 +68,10 @@ public PatternMatchingTranslator(
             Source source,
             ParserContext parserContext,
             Node currentNode,
-            ParseNode data, // data to match on
             ListParseNode cases, // cases to check
             TranslatorEnvironment environment,
             BodyTranslator bodyTranslator) {
         super(language, source, parserContext, currentNode, environment);
-        this.data = data;
         this.cases = cases;
         this.environment = environment;
         this.bodyTranslator = bodyTranslator;