From 54298c21e3ef18e76e57c4ac6f504052d5dca985 Mon Sep 17 00:00:00 2001 From: benmurphyy <47269122+benmurphyy@users.noreply.github.com> Date: Wed, 31 Jan 2024 22:42:42 +0800 Subject: [PATCH] Fix conditional expressions short circuit behaviour --- ctowasm/src/processor/processBlockItem.ts | 11 +++++++- ctowasm/src/translator/translateExpression.ts | 3 ++- ctowasm/src/translator/wasm-ast/control.ts | 1 + ctowasm/src/translator/wasm-ast/core.ts | 5 ++-- .../src/translator/wasm-ast/expressions.ts | 10 ++++--- .../wat-generator/generateWatExpression.ts | 6 ++--- .../subset5/valid/conditional_expression.c | 26 +++++++++++++++++++ .../subset5/valid/conditional_expression_1.c | 7 ----- ctowasm/test/testLog.js | 8 ++++++ 9 files changed, 59 insertions(+), 18 deletions(-) create mode 100644 ctowasm/test/samples/subset5/valid/conditional_expression.c delete mode 100644 ctowasm/test/samples/subset5/valid/conditional_expression_1.c diff --git a/ctowasm/src/processor/processBlockItem.ts b/ctowasm/src/processor/processBlockItem.ts index d4cf3f75..dfd2483e 100644 --- a/ctowasm/src/processor/processBlockItem.ts +++ b/ctowasm/src/processor/processBlockItem.ts @@ -166,6 +166,14 @@ export default function processBlockItem( ); }); return processedExpressions; + } else if (node.type === "ConditionalExpression") { + // break this conditional into a simple if else expression (expressions inside condtional may have side effects) + return [{ + type: "SelectionStatement", + condition: processCondition(node.condition, symbolTable), + ifStatements: processBlockItem(node.trueExpression, symbolTable, enclosingFunc), + elseStatements: processBlockItem(node.falseExpression, symbolTable, enclosingFunc) + }] } else if ( node.type === "AddressOfExpression" || node.type === "BinaryExpression" || @@ -173,7 +181,8 @@ export default function processBlockItem( node.type === "IntegerConstant" || node.type === "IdentifierExpression" || node.type === "PointerDereference" || - node.type === "SizeOfExpression" + node.type === "SizeOfExpression" || + node.type === "StructMemberAccess" ) { // all these expression statements can be safely ignored as they have no side effects return []; diff --git a/ctowasm/src/translator/translateExpression.ts b/ctowasm/src/translator/translateExpression.ts index 964255d0..86391f0b 100644 --- a/ctowasm/src/translator/translateExpression.ts +++ b/ctowasm/src/translator/translateExpression.ts @@ -119,10 +119,11 @@ export default function translateExpression( }; } else if(expr.type === "ConditionalExpression") { return { - type: "SelectExpression", + type: "ConditionalExpression", condition: createWasmBooleanExpression(expr.condition), trueExpression: translateExpression(expr.trueExpression, expr.dataType, enclosingLoopDetails), falseExpression: translateExpression(expr.falseExpression, expr.dataType, enclosingLoopDetails), + wasmDataType: convertScalarDataTypeToWasmType(expr.dataType) } } else { throw new TranslationError(`Unhandled expression: ${toJson(expr)}`); diff --git a/ctowasm/src/translator/wasm-ast/control.ts b/ctowasm/src/translator/wasm-ast/control.ts index 519aaffc..860caa8f 100644 --- a/ctowasm/src/translator/wasm-ast/control.ts +++ b/ctowasm/src/translator/wasm-ast/control.ts @@ -3,6 +3,7 @@ import { WasmExpression, WasmStatement, } from "~src/translator/wasm-ast/core"; +import { WasmDataType } from "~src/translator/wasm-ast/dataTypes"; import { WasmBooleanExpression } from "~src/translator/wasm-ast/expressions"; /** diff --git a/ctowasm/src/translator/wasm-ast/core.ts b/ctowasm/src/translator/wasm-ast/core.ts index b6af25c4..0ab5bca4 100644 --- a/ctowasm/src/translator/wasm-ast/core.ts +++ b/ctowasm/src/translator/wasm-ast/core.ts @@ -8,10 +8,10 @@ import { } from "~src/translator/wasm-ast/control"; import { WasmBinaryExpression, + WasmConditionalExpression, WasmNegateFloatExpression, WasmPostStatementExpression, WasmPreStatementExpression, - WasmSelectExpression, } from "~src/translator/wasm-ast/expressions"; import { WasmFunction, @@ -84,4 +84,5 @@ export type WasmExpression = | WasmGlobalGet | WasmPreStatementExpression | WasmPostStatementExpression - | WasmSelectExpression; + | WasmConditionalExpression + diff --git a/ctowasm/src/translator/wasm-ast/expressions.ts b/ctowasm/src/translator/wasm-ast/expressions.ts index 93304e70..1f26cf3c 100644 --- a/ctowasm/src/translator/wasm-ast/expressions.ts +++ b/ctowasm/src/translator/wasm-ast/expressions.ts @@ -61,11 +61,13 @@ export interface WasmBooleanExpression extends WasmWrapperNode { } /** - * Wasm's version of a conditional expression e.g. 1 ? 2 : 3 + * Custom WasmConditionalExpressin (comprised of multiple basic Wasm nodes as Wasm does not have native support for such a construct) + * Wasm Generator will convert this into a if else block that returns the given type. */ -export interface WasmSelectExpression extends WasmAstNode { - type: "SelectExpression"; - condition: WasmExpression; +export interface WasmConditionalExpression extends WasmAstNode { + type: "ConditionalExpression"; + condition: WasmBooleanExpression; trueExpression: WasmExpression; falseExpression: WasmExpression; + wasmDataType: WasmDataType; } diff --git a/ctowasm/src/wat-generator/generateWatExpression.ts b/ctowasm/src/wat-generator/generateWatExpression.ts index 8b993000..9fe85fee 100644 --- a/ctowasm/src/wat-generator/generateWatExpression.ts +++ b/ctowasm/src/wat-generator/generateWatExpression.ts @@ -52,9 +52,9 @@ export default function generateWatExpression(node: WasmExpression): string { return `${generateStatementsList(node.statements)} ${generateWatExpression( node.expr, )}`; - } else if (node.type === "SelectExpression") { - return `(select ${generateWatExpression(node.trueExpression)} ${generateWatExpression(node.falseExpression)} ${generateWatExpression(node.condition)})` - }else { + } else if (node.type === "ConditionalExpression") { + return `(if (result ${node.wasmDataType}) ${generateWatExpression(node.condition)} (then ${generateWatExpression(node.trueExpression)}) (else ${generateWatExpression(node.falseExpression)}))` + } else { throw new WatGeneratorError(`Unhandled WAT AST node: ${toJson(node)}`); } } diff --git a/ctowasm/test/samples/subset5/valid/conditional_expression.c b/ctowasm/test/samples/subset5/valid/conditional_expression.c new file mode 100644 index 00000000..dcd33a5e --- /dev/null +++ b/ctowasm/test/samples/subset5/valid/conditional_expression.c @@ -0,0 +1,26 @@ +// Test conditional expressions +#include + +int f() { + print_int(-1); + return -1; +} + +int main() { + int x = 1 ? 2 : 3; + print_int(x); + + // test that implicit arithmetic conversion works + long y = 100; + print_long(x == 2 ? y : x); + + // test that short circuiting works + int a = 1 ? 1 : f(); // f() should not be called + int b = 0 ? f() : 2; // f() should not be called + print_int(a); + print_int(b); + + // test that conditionals can be used as statements and side effects will be included + 1 ? f() : f(); + 0 ? f() : f(); +} \ No newline at end of file diff --git a/ctowasm/test/samples/subset5/valid/conditional_expression_1.c b/ctowasm/test/samples/subset5/valid/conditional_expression_1.c deleted file mode 100644 index 23843a3a..00000000 --- a/ctowasm/test/samples/subset5/valid/conditional_expression_1.c +++ /dev/null @@ -1,7 +0,0 @@ -// Test conditional expressions -#include - -int main() { - int x = 1 ? 2 : 3; - print_int(x); -} \ No newline at end of file diff --git a/ctowasm/test/testLog.js b/ctowasm/test/testLog.js index 229b3f2d..669eb216 100644 --- a/ctowasm/test/testLog.js +++ b/ctowasm/test/testLog.js @@ -572,6 +572,14 @@ const testLog = { 12, 7, 14, 8, 16, 9, 18, ], }, + conditional_expression: { + title: "Test conditional expressions", + expectedCode: false, + expectedValues: [ + 2, 100, 1, 2, -1 , -1 + ] + } + }, };