Skip to content

Commit

Permalink
Fix conditional expressions short circuit behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
benmurphyy committed Jan 31, 2024
1 parent a6f5a8f commit 54298c2
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 18 deletions.
11 changes: 10 additions & 1 deletion ctowasm/src/processor/processBlockItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,23 @@ 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" ||
node.type === "FloatConstant" ||
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 [];
Expand Down
3 changes: 2 additions & 1 deletion ctowasm/src/translator/translateExpression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`);
Expand Down
1 change: 1 addition & 0 deletions ctowasm/src/translator/wasm-ast/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand Down
5 changes: 3 additions & 2 deletions ctowasm/src/translator/wasm-ast/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -84,4 +84,5 @@ export type WasmExpression =
| WasmGlobalGet
| WasmPreStatementExpression
| WasmPostStatementExpression
| WasmSelectExpression;
| WasmConditionalExpression

10 changes: 6 additions & 4 deletions ctowasm/src/translator/wasm-ast/expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
6 changes: 3 additions & 3 deletions ctowasm/src/wat-generator/generateWatExpression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`);
}
}
26 changes: 26 additions & 0 deletions ctowasm/test/samples/subset5/valid/conditional_expression.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Test conditional expressions
#include <source_stdlib>

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();
}
7 changes: 0 additions & 7 deletions ctowasm/test/samples/subset5/valid/conditional_expression_1.c

This file was deleted.

8 changes: 8 additions & 0 deletions ctowasm/test/testLog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
]
}

},
};

Expand Down

0 comments on commit 54298c2

Please sign in to comment.