Skip to content

Commit 0af8cce

Browse files
mattjohnsonpintCountBleck
authored andcommitted
revise to re-use existing equality comparison code
1 parent f523d13 commit 0af8cce

File tree

4 files changed

+3510
-996
lines changed

4 files changed

+3510
-996
lines changed

src/compiler.ts

Lines changed: 66 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2817,48 +2817,18 @@ export class Compiler extends DiagnosticEmitter {
28172817

28182818
// Compile the condition (always executes)
28192819
let condExpr = this.compileExpression(statement.condition, Type.auto);
2820-
2821-
// Get the type set from compiling the condition expression
2822-
let currentType = this.currentType;
2820+
let condType = this.currentType;
28232821

2824-
// Determine the binary operation to use for comparison
2825-
let binaryOp: BinaryOp;
2826-
switch (currentType.toRef()) {
2827-
case TypeRef.I32: {
2828-
binaryOp = BinaryOp.EqI32;
2829-
break;
2830-
}
2831-
case TypeRef.I64: {
2832-
binaryOp = BinaryOp.EqI64;
2833-
break;
2834-
}
2835-
case TypeRef.F32: {
2836-
binaryOp = BinaryOp.EqF32;
2837-
break;
2838-
}
2839-
case TypeRef.F64: {
2840-
binaryOp = BinaryOp.EqF64;
2841-
break;
2842-
}
2843-
default: {
2844-
this.error(
2845-
DiagnosticCode.Not_implemented_0,
2846-
statement.range, `Switch condition of type ${currentType}`
2847-
);
2848-
return module.unreachable();
2849-
}
2850-
}
2851-
28522822
// Shortcut if there are no cases
28532823
if (!numCases) return module.drop(condExpr);
28542824

28552825
// Assign the condition to a temporary local as we compare it multiple times
28562826
let outerFlow = this.currentFlow;
2857-
let tempLocal = outerFlow.getTempLocal(currentType);
2827+
let tempLocal = outerFlow.getTempLocal(condType);
28582828
let tempLocalIndex = tempLocal.index;
28592829
let breaks = new Array<ExpressionRef>(1 + numCases);
2860-
breaks[0] = module.local_set(tempLocalIndex, condExpr, currentType.isManaged);
2861-
2830+
breaks[0] = module.local_set(tempLocalIndex, condExpr, condType.isManaged);
2831+
28622832
// Make one br_if per labeled case and leave it to Binaryen to optimize the
28632833
// sequence of br_ifs to a br_table according to optimization levels
28642834
let breakIndex = 1;
@@ -2870,14 +2840,24 @@ export class Compiler extends DiagnosticEmitter {
28702840
defaultIndex = i;
28712841
continue;
28722842
}
2873-
breaks[breakIndex++] = module.br(`case${i}|${label}`,
2874-
module.binary(binaryOp,
2875-
module.local_get(tempLocalIndex, currentType.toRef()),
2876-
this.compileExpression(assert(case_.label), currentType,
2877-
Constraints.ConvImplicit
2878-
)
2879-
)
2843+
2844+
// Compile the equality expression for this case
2845+
const left = statement.condition;
2846+
const leftExpr = module.local_get(tempLocalIndex, condType.toRef());
2847+
const leftType = condType;
2848+
const right = case_.label!;
2849+
const rightExpr = this.compileExpression(assert(case_.label), condType, Constraints.ConvImplicit);
2850+
const rightType = this.currentType;
2851+
const equalityExpr = this.compileCommutativeCompareBinaryExpressionFromParts(
2852+
Token.Equals_Equals,
2853+
left, leftExpr, leftType,
2854+
right, rightExpr, rightType,
2855+
condType,
2856+
statement
28802857
);
2858+
2859+
// Add it to the list of breaks
2860+
breaks[breakIndex++] = module.br(`case${i}|${label}`, equalityExpr);
28812861
}
28822862

28832863
// If there is a default case, break to it, otherwise break out of the switch
@@ -3829,32 +3809,53 @@ export class Compiler extends DiagnosticEmitter {
38293809
expression: BinaryExpression,
38303810
contextualType: Type,
38313811
): ExpressionRef {
3832-
let module = this.module;
3833-
let left = expression.left;
3834-
let right = expression.right;
3812+
3813+
const left = expression.left;
3814+
const leftExpr = this.compileExpression(left, contextualType);
3815+
const leftType = this.currentType;
3816+
3817+
const right = expression.right;
3818+
const rightExpr = this.compileExpression(right, leftType);
3819+
const rightType = this.currentType;
3820+
3821+
return this.compileCommutativeCompareBinaryExpressionFromParts(
3822+
expression.operator,
3823+
left, leftExpr, leftType,
3824+
right, rightExpr, rightType,
3825+
contextualType,
3826+
expression
3827+
);
3828+
}
38353829

3836-
let leftExpr: ExpressionRef;
3837-
let leftType: Type;
3838-
let rightExpr: ExpressionRef;
3839-
let rightType: Type;
3840-
let commonType: Type | null;
3830+
/**
3831+
* compile `==` `===` `!=` `!==` BinaryExpression, from previously compiled left and right expressions.
3832+
*
3833+
* This is split from `compileCommutativeCompareBinaryExpression` so that the logic can be reused
3834+
* for switch cases in `compileSwitchStatement`, where the left expression only should be compiled once.
3835+
*/
3836+
private compileCommutativeCompareBinaryExpressionFromParts(
3837+
operator: Token,
3838+
left: Expression,
3839+
leftExpr: ExpressionRef,
3840+
leftType: Type,
3841+
right: Expression,
3842+
rightExpr: ExpressionRef,
3843+
rightType: Type,
3844+
contextualType: Type,
3845+
reportNode: Node
3846+
): ExpressionRef {
38413847

3842-
let operator = expression.operator;
3848+
let module = this.module;
38433849
let operatorString = operatorTokenToString(operator);
3844-
3845-
leftExpr = this.compileExpression(left, contextualType);
3846-
leftType = this.currentType;
3847-
3848-
rightExpr = this.compileExpression(right, leftType);
3849-
rightType = this.currentType;
38503850

38513851
// check operator overload
38523852
const operatorKind = OperatorKind.fromBinaryToken(operator);
38533853
const leftOverload = leftType.lookupOverload(operatorKind, this.program);
38543854
const rightOverload = rightType.lookupOverload(operatorKind, this.program);
38553855
if (leftOverload && rightOverload && leftOverload != rightOverload) {
38563856
this.error(
3857-
DiagnosticCode.Ambiguous_operator_overload_0_conflicting_overloads_1_and_2, expression.range,
3857+
DiagnosticCode.Ambiguous_operator_overload_0_conflicting_overloads_1_and_2,
3858+
reportNode.range,
38583859
operatorString,
38593860
leftOverload.internalName,
38603861
rightOverload.internalName
@@ -3867,23 +3868,23 @@ export class Compiler extends DiagnosticEmitter {
38673868
leftOverload,
38683869
left, leftExpr, leftType,
38693870
right, rightExpr, rightType,
3870-
expression
3871+
reportNode
38713872
);
38723873
}
38733874
if (rightOverload) {
38743875
return this.compileCommutativeBinaryOverload(
38753876
rightOverload,
38763877
right, rightExpr, rightType,
38773878
left, leftExpr, leftType,
3878-
expression
3879+
reportNode
38793880
);
38803881
}
38813882
const signednessIsRelevant = false;
3882-
commonType = Type.commonType(leftType, rightType, contextualType, signednessIsRelevant);
3883+
const commonType = Type.commonType(leftType, rightType, contextualType, signednessIsRelevant);
38833884
if (!commonType) {
38843885
this.error(
38853886
DiagnosticCode.Operator_0_cannot_be_applied_to_types_1_and_2,
3886-
expression.range,
3887+
reportNode.range,
38873888
operatorString,
38883889
leftType.toString(),
38893890
rightType.toString()
@@ -3896,13 +3897,13 @@ export class Compiler extends DiagnosticEmitter {
38963897
if (isConstExpressionNaN(module, rightExpr) || isConstExpressionNaN(module, leftExpr)) {
38973898
this.warning(
38983899
DiagnosticCode._NaN_does_not_compare_equal_to_any_other_value_including_itself_Use_isNaN_x_instead,
3899-
expression.range
3900+
reportNode.range
39003901
);
39013902
}
39023903
if (isConstNegZero(rightExpr) || isConstNegZero(leftExpr)) {
39033904
this.warning(
39043905
DiagnosticCode.Comparison_with_0_0_is_sign_insensitive_Use_Object_is_x_0_0_if_the_sign_matters,
3905-
expression.range
3906+
reportNode.range
39063907
);
39073908
}
39083909
}
@@ -3916,10 +3917,10 @@ export class Compiler extends DiagnosticEmitter {
39163917
switch (operator) {
39173918
case Token.Equals_Equals_Equals:
39183919
case Token.Equals_Equals:
3919-
return this.makeEq(leftExpr, rightExpr, commonType, expression);
3920+
return this.makeEq(leftExpr, rightExpr, commonType, reportNode);
39203921
case Token.Exclamation_Equals_Equals:
39213922
case Token.Exclamation_Equals:
3922-
return this.makeNe(leftExpr, rightExpr, commonType, expression);
3923+
return this.makeNe(leftExpr, rightExpr, commonType, reportNode);
39233924
default:
39243925
assert(false);
39253926
return module.unreachable();

0 commit comments

Comments
 (0)