From d8b2af11c8f1c6a022735f912aba7b9d7426a014 Mon Sep 17 00:00:00 2001 From: Xavier Dufour Date: Sat, 18 Jan 2025 21:40:37 -0500 Subject: [PATCH 1/5] Updated _sh/switch.c for A/B testing, updated sh.c --- sh.c | 46 +++++++++++++++++++++++++++++----------- tests/_exe/switch.c | 12 +++++++++++ tests/_exe/switch.golden | 1 + tests/_sh/switch.c | 12 +++++++++++ tests/_sh/switch.golden | 2 +- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/sh.c b/sh.c index 5a582340..de3a8bcc 100644 --- a/sh.c +++ b/sh.c @@ -1789,23 +1789,18 @@ bool comp_switch(ast node) { node = get_child(node, 1); - if (node == 0 || get_op(node) != '{') fatal_error("comp_statement: switch without body"); + if (node == 0 || (get_op(node) != '{' && get_op(node) != CASE_KW)) fatal_error("comp_statement: switch without body"); - while (get_op(node) == '{') { - statement = get_child(node, 0); - node = get_child(node, 1); + // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > is a single < case constant-expression : statement > + if(get_op(node) == CASE_KW) { + // There is no child to fetch, we are already at the case keyword + append_glo_decl(make_switch_pattern(node)); - append_glo_decl(make_switch_pattern(statement)); statement = last_stmt; // last_stmt is set by make_switch_pattern - nest_level += 1; - // Since we don't know if the switch is exhaustive, we can't compile in tail - // position mode, see comment below. in_tail_position = false; - - // We keep compiling statements until we encounter a statement that returns or breaks. - // Case and default nodes contain the first statement of the block so we process that one first. + if (!comp_statement(statement, STMT_CTX_SWITCH)) { while (get_op(node) == '{') { statement = get_child(node, 0); @@ -1813,10 +1808,37 @@ bool comp_switch(ast node) { if (comp_statement(statement, STMT_CTX_SWITCH)) break; } } - nest_level -= 1; append_glo_decl(wrap_str_lit(";;")); } + else { + while (get_op(node) == '{') { + statement = get_child(node, 0); + node = get_child(node, 1); + + append_glo_decl(make_switch_pattern(statement)); + statement = last_stmt; // last_stmt is set by make_switch_pattern + + nest_level += 1; + + // Since we don't know if the switch is exhaustive, we can't compile in tail + // position mode, see comment below. + in_tail_position = false; + + // We keep compiling statements until we encounter a statement that returns or breaks. + // Case and default nodes contain the first statement of the block so we process that one first. + if (!comp_statement(statement, STMT_CTX_SWITCH)) { + while (get_op(node) == '{') { + statement = get_child(node, 0); + node = get_child(node, 1); + if (comp_statement(statement, STMT_CTX_SWITCH)) break; + } + } + + nest_level -= 1; + append_glo_decl(wrap_str_lit(";;")); + } + } nest_level -= 1; append_glo_decl(wrap_str_lit("esac")); diff --git a/tests/_exe/switch.c b/tests/_exe/switch.c index 4daf5cd8..f6e0f69c 100644 --- a/tests/_exe/switch.c +++ b/tests/_exe/switch.c @@ -206,6 +206,17 @@ void state_machine_switch() { } } +void bodiless_switch() { + switch (123) + case 123: + putchar('E'); + + switch (456) + case 123: + case 456: + putchar('E'); +} + int main() { empty_switch(); putchar('\n'); no_case_switch(); putchar('\n'); @@ -220,5 +231,6 @@ int main() { duff_device_switch(15); putchar('\n'); state_machine_switch(); state_machine_switch(); putchar('\n'); + bodiless_switch(); putchar('\n'); return 0; } diff --git a/tests/_exe/switch.golden b/tests/_exe/switch.golden index e46c6432..492e0907 100644 --- a/tests/_exe/switch.golden +++ b/tests/_exe/switch.golden @@ -10,3 +10,4 @@ ABCDE DT BCDEFGHABCDEFGH ABA +EE diff --git a/tests/_sh/switch.c b/tests/_sh/switch.c index ba57458e..8f4cdade 100644 --- a/tests/_sh/switch.c +++ b/tests/_sh/switch.c @@ -93,9 +93,21 @@ void switch_in_while(){ } } +void bodiless_switch() { + switch (123) + case 123: + putchar('F'); + + switch (456) + case 123: + case 456: + putchar('E'); +} + void main() { simple_switch(); simple_switch_fall_through(); multiple_labels(); switch_in_while(); + bodiless_switch(); } diff --git a/tests/_sh/switch.golden b/tests/_sh/switch.golden index ff841f31..3134ac1d 100644 --- a/tests/_sh/switch.golden +++ b/tests/_sh/switch.golden @@ -1 +1 @@ -BCCBABCDE \ No newline at end of file +BCCBABCDEFE \ No newline at end of file From d3c9e3f82f1a51959d146f7885e843e00c9f2f0c Mon Sep 17 00:00:00 2001 From: Xavier Dufour Date: Sat, 18 Jan 2025 22:27:04 -0500 Subject: [PATCH 2/5] Refactored to reduce copypaste in the if statement --- sh.c | 52 +++++++++++++++++++--------------------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/sh.c b/sh.c index de3a8bcc..c995fcb9 100644 --- a/sh.c +++ b/sh.c @@ -1791,16 +1791,29 @@ bool comp_switch(ast node) { if (node == 0 || (get_op(node) != '{' && get_op(node) != CASE_KW)) fatal_error("comp_statement: switch without body"); - // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > is a single < case constant-expression : statement > - if(get_op(node) == CASE_KW) { - // There is no child to fetch, we are already at the case keyword - append_glo_decl(make_switch_pattern(node)); + while (get_op(node) == '{' || get_op(node) == CASE_KW) { + if(get_op(node) == CASE_KW) { + // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > + // is a single < case constant-expression : statement > + statement = node; + node = get_child(node, 0); + } + else { + statement = get_child(node, 0); + node = get_child(node, 1); + } + append_glo_decl(make_switch_pattern(statement)); statement = last_stmt; // last_stmt is set by make_switch_pattern + nest_level += 1; + // Since we don't know if the switch is exhaustive, we can't compile in tail + // position mode, see comment below. in_tail_position = false; - + + // We keep compiling statements until we encounter a statement that returns or breaks. + // Case and default nodes contain the first statement of the block so we process that one first. if (!comp_statement(statement, STMT_CTX_SWITCH)) { while (get_op(node) == '{') { statement = get_child(node, 0); @@ -1808,37 +1821,10 @@ bool comp_switch(ast node) { if (comp_statement(statement, STMT_CTX_SWITCH)) break; } } + nest_level -= 1; append_glo_decl(wrap_str_lit(";;")); } - else { - while (get_op(node) == '{') { - statement = get_child(node, 0); - node = get_child(node, 1); - - append_glo_decl(make_switch_pattern(statement)); - statement = last_stmt; // last_stmt is set by make_switch_pattern - - nest_level += 1; - - // Since we don't know if the switch is exhaustive, we can't compile in tail - // position mode, see comment below. - in_tail_position = false; - - // We keep compiling statements until we encounter a statement that returns or breaks. - // Case and default nodes contain the first statement of the block so we process that one first. - if (!comp_statement(statement, STMT_CTX_SWITCH)) { - while (get_op(node) == '{') { - statement = get_child(node, 0); - node = get_child(node, 1); - if (comp_statement(statement, STMT_CTX_SWITCH)) break; - } - } - - nest_level -= 1; - append_glo_decl(wrap_str_lit(";;")); - } - } nest_level -= 1; append_glo_decl(wrap_str_lit("esac")); From cce0332413084b9eb9d36e959003b1125a539c78 Mon Sep 17 00:00:00 2001 From: Xavier Dufour Date: Sun, 19 Jan 2025 18:51:45 -0500 Subject: [PATCH 3/5] Changes following PR comments --- sh.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sh.c b/sh.c index c995fcb9..617e3969 100644 --- a/sh.c +++ b/sh.c @@ -1791,17 +1791,15 @@ bool comp_switch(ast node) { if (node == 0 || (get_op(node) != '{' && get_op(node) != CASE_KW)) fatal_error("comp_statement: switch without body"); - while (get_op(node) == '{' || get_op(node) == CASE_KW) { - if(get_op(node) == CASE_KW) { - // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > - // is a single < case constant-expression : statement > - statement = node; - node = get_child(node, 0); - } - else { - statement = get_child(node, 0); - node = get_child(node, 1); - } + if(get_op(node) == CASE_KW) { + // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > + // is a single < case constant-expression : statement > + // therefore we wrap the case statement with a block statement to simplify down to the typical syntax + node = new_ast2('{', node, 0); + } + while (get_op(node) == '{') { + statement = get_child(node, 0); + node = get_child(node, 1); append_glo_decl(make_switch_pattern(statement)); statement = last_stmt; // last_stmt is set by make_switch_pattern From 4cdaa5c6f0b04bd7accd5aa45a9d1e8d1c95f20a Mon Sep 17 00:00:00 2001 From: Xavier Dufour Date: Thu, 30 Jan 2025 17:39:24 -0500 Subject: [PATCH 4/5] Fix trailing whitespaces, moved the edge case check to reduce null check --- sh.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sh.c b/sh.c index 617e3969..da4087a7 100644 --- a/sh.c +++ b/sh.c @@ -1789,14 +1789,14 @@ bool comp_switch(ast node) { node = get_child(node, 1); - if (node == 0 || (get_op(node) != '{' && get_op(node) != CASE_KW)) fatal_error("comp_statement: switch without body"); - if(get_op(node) == CASE_KW) { - // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > + // This is for the edge case where the entire 'statement' part of < switch ( expression ) statement > // is a single < case constant-expression : statement > // therefore we wrap the case statement with a block statement to simplify down to the typical syntax node = new_ast2('{', node, 0); } + + if (node == 0 || get_op(node) != '{') fatal_error("comp_statement: switch without body"); while (get_op(node) == '{') { statement = get_child(node, 0); node = get_child(node, 1); From 263d4237e6bf79a41c795d4481220a9cc3b90772 Mon Sep 17 00:00:00 2001 From: Laurent Huberdeau Date: Wed, 19 Feb 2025 17:55:12 -0500 Subject: [PATCH 5/5] Remove trailing whitespace in tests --- tests/_exe/switch.c | 4 ++-- tests/_sh/switch.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/_exe/switch.c b/tests/_exe/switch.c index 6709f5f2..843f727a 100644 --- a/tests/_exe/switch.c +++ b/tests/_exe/switch.c @@ -217,10 +217,10 @@ void state_machine_switch() { void bodiless_switch() { switch (123) - case 123: + case 123: putchar('E'); - switch (456) + switch (456) case 123: case 456: putchar('E'); diff --git a/tests/_sh/switch.c b/tests/_sh/switch.c index 8f4cdade..4f4b5ba0 100644 --- a/tests/_sh/switch.c +++ b/tests/_sh/switch.c @@ -94,11 +94,11 @@ void switch_in_while(){ } void bodiless_switch() { - switch (123) - case 123: + switch (123) + case 123: putchar('F'); - switch (456) + switch (456) case 123: case 456: putchar('E');