From 05b9b58d554042aef68462c727ae97325f986961 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Mon, 2 Oct 2023 17:51:14 +0100 Subject: [PATCH] [Prism] Fix IfNode and ElseNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ElseNode looks to have been implemented at the same time as IfNode, but was resulting in a stack underflow error. The following is from the test code ``` if foo bar end ``` ``` ❯ make run compiling compile.c linking miniruby ./miniruby -I./lib -I. -I.ext/common -r./arm64-darwin22-fake ./test.rb CRUBY: ************************************************** == disasm: #@:1 (1,0)-(2,19)> 0000 putself ( 2)[Li] 0001 opt_send_without_block 0003 branchunless 9 0005 putself 0006 opt_send_without_block 0008 pop 0009 putobject_INT2FIX_1_ 0010 leave PRISM: ************************************************** -- raw disasm-------- 0000 putself ( 2) 0001 send , nil ( 2) 0004 branchunless ( 2) 0006 jump ( 2) [sp: 0] * 0008 pop ( 1) 0009 putself ( 2) 0010 send , nil ( 2) 0013 pop ( 2) 0014 jump ( 1) [sp: 0] [sp: -1] 0016 putobject 1 ( 2) 0018 leave ( 1) --------------------- : :1: argument stack underflow (-1) (SyntaxError) make: *** [run] Error 1 ``` This commit fixes the stack underflow error for both IfNode and ElseNode and introduces tests for them. --- prism_compile.c | 9 ++++++--- test/ruby/test_compile_prism.rb | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/prism_compile.c b/prism_compile.c index a7b6c14ce4..4747ca5b0f 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -407,7 +407,6 @@ pm_compile_if(rb_iseq_t *iseq, const int line, pm_statements_node_t *node_body, INIT_ANCHOR(then_seq); if (node_body) { pm_compile_node(iseq, (pm_node_t *)node_body, then_seq, src, popped, compile_context); - PM_POP_IF_POPPED; } else { PM_PUTNIL_UNLESS_POPPED; } @@ -415,7 +414,6 @@ pm_compile_if(rb_iseq_t *iseq, const int line, pm_statements_node_t *node_body, if (else_label->refcnt) { end_label = NEW_LABEL(line); ADD_INSNL(then_seq, &dummy_line_node, jump, end_label); - PM_POP_UNLESS_POPPED; } ADD_SEQ(ret, then_seq); } @@ -426,7 +424,7 @@ pm_compile_if(rb_iseq_t *iseq, const int line, pm_statements_node_t *node_body, DECL_ANCHOR(else_seq); INIT_ANCHOR(else_seq); if (node_else) { - pm_compile_node(iseq, (pm_node_t *)(((pm_else_node_t *)node_else)->statements), else_seq, src, popped, compile_context); + pm_compile_node(iseq, (pm_node_t *)node_else, else_seq, src, popped, compile_context); } else { PM_PUTNIL_UNLESS_POPPED; @@ -1365,6 +1363,11 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, ADD_INSN1(ret, &dummy_line_node, putobject, Qfalse); } return; + case PM_ELSE_NODE: { + pm_else_node_t *cast = (pm_else_node_t *)node; + PM_COMPILE((pm_node_t *)cast->statements); + return; + } case PM_FLIP_FLOP_NODE: { // TODO: The labels here are wrong, figure out why..... pm_flip_flop_node_t *flip_flop_node = (pm_flip_flop_node_t *)node; diff --git a/test/ruby/test_compile_prism.rb b/test/ruby/test_compile_prism.rb index 9c2f232396..f0e389d30a 100644 --- a/test/ruby/test_compile_prism.rb +++ b/test/ruby/test_compile_prism.rb @@ -338,6 +338,18 @@ module Prism test_prism_eval("false || 1") end + def test_IfNode + test_prism_eval("if true; 1; end") + test_prism_eval("1 if true") + end + + def test_ElseNode + test_prism_eval("if false; 0; else; 1; end") + test_prism_eval("if true; 0; else; 1; end") + test_prism_eval("true ? 1 : 0") + test_prism_eval("false ? 0 : 1") + end + ############################################################################ # Calls / arugments # ############################################################################