Skip to content

Commit 9d8d4fa

Browse files
MaxGraeyMaxGraey
authored
fix: Fix non-nullable assertion for externref-s (#2337)
1 parent ffbcbb2 commit 9d8d4fa

9 files changed

+157
-60
lines changed

Diff for: ‎src/builtins.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3119,7 +3119,7 @@ function builtin_memory_data(ctx: BuiltinContext): ExpressionRef {
31193119
}
31203120
exprs[i] = expr;
31213121
} else {
3122-
exprs[i] = compiler.makeZero(elementType, elementExpression);
3122+
exprs[i] = compiler.makeZero(elementType);
31233123
}
31243124
}
31253125
if (!isStatic) {

Diff for: ‎src/compiler.ts

+53-27
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import {
2525
ExpressionRef,
2626
UnaryOp,
2727
BinaryOp,
28-
RefIsOp,
2928
TypeRef,
3029
FunctionRef,
3130
ExpressionId,
@@ -1245,7 +1244,7 @@ export class Compiler extends DiagnosticEmitter {
12451244
if (global.is(CommonFlags.INLINED)) {
12461245
initExpr = this.compileInlineConstant(global, global.type, Constraints.PREFER_STATIC);
12471246
} else {
1248-
initExpr = this.makeZero(type, global.declaration);
1247+
initExpr = this.makeZero(type);
12491248
}
12501249
}
12511250

@@ -1258,7 +1257,7 @@ export class Compiler extends DiagnosticEmitter {
12581257
findDecorator(DecoratorKind.INLINE, global.decoratorNodes)!.range, "inline"
12591258
);
12601259
}
1261-
module.addGlobal(internalName, typeRef, true, this.makeZero(type, global.declaration));
1260+
module.addGlobal(internalName, typeRef, true, this.makeZero(type));
12621261
this.currentBody.push(
12631262
module.global_set(internalName, initExpr)
12641263
);
@@ -7041,7 +7040,7 @@ export class Compiler extends DiagnosticEmitter {
70417040
let needsVarargsStub = false;
70427041
for (let n = numParameters; n < overloadNumParameters; ++n) {
70437042
// TODO: inline constant initializers and skip varargs stub
7044-
paramExprs[1 + n] = this.makeZero(overloadParameterTypes[n], overloadInstance.declaration);
7043+
paramExprs[1 + n] = this.makeZero(overloadParameterTypes[n]);
70457044
needsVarargsStub = true;
70467045
}
70477046
let calledName = needsVarargsStub
@@ -7241,7 +7240,7 @@ export class Compiler extends DiagnosticEmitter {
72417240
}
72427241
}
72437242
}
7244-
operands.push(this.makeZero(parameterTypes[i], instance.declaration));
7243+
operands.push(this.makeZero(parameterTypes[i]));
72457244
allOptionalsAreConstant = false;
72467245
}
72477246
if (!allOptionalsAreConstant && !instance.is(CommonFlags.MODULE_IMPORT)) {
@@ -7351,7 +7350,7 @@ export class Compiler extends DiagnosticEmitter {
73517350
}
73527351
let parameterTypes = signature.parameterTypes;
73537352
for (let i = numArguments; i < maxArguments; ++i) {
7354-
operands.push(this.makeZero(parameterTypes[i], reportNode));
7353+
operands.push(this.makeZero(parameterTypes[i]));
73557354
}
73567355
}
73577356

@@ -7645,7 +7644,7 @@ export class Compiler extends DiagnosticEmitter {
76457644
this.currentType = signatureReference.type.asNullable();
76467645
return options.isWasm64 ? module.i64(0) : module.i32(0);
76477646
}
7648-
return this.makeZero(contextualType, expression);
7647+
return this.makeZero(contextualType);
76497648
}
76507649
this.currentType = options.usizeType;
76517650
this.warning(
@@ -7937,7 +7936,7 @@ export class Compiler extends DiagnosticEmitter {
79377936
? BinaryOp.NeI64
79387937
: BinaryOp.NeI32,
79397938
expr,
7940-
this.makeZero(actualType, expression.expression)
7939+
this.makeZero(actualType)
79417940
);
79427941
}
79437942

@@ -8044,7 +8043,7 @@ export class Compiler extends DiagnosticEmitter {
80448043
? BinaryOp.NeI64
80458044
: BinaryOp.NeI32,
80468045
expr,
8047-
this.makeZero(actualType, expression.expression)
8046+
this.makeZero(actualType)
80488047
);
80498048

80508049
// <nonNullable> is just `true`
@@ -8395,7 +8394,7 @@ export class Compiler extends DiagnosticEmitter {
83958394
}
83968395
values[i] = expr;
83978396
} else {
8398-
values[i] = this.makeZero(elementType, elementExpression);
8397+
values[i] = this.makeZero(elementType);
83998398
}
84008399
}
84018400

@@ -8547,7 +8546,7 @@ export class Compiler extends DiagnosticEmitter {
85478546
}
85488547
values[i] = expr;
85498548
} else {
8550-
values[i] = this.makeZero(elementType, elementExpression);
8549+
values[i] = this.makeZero(elementType);
85518550
}
85528551
}
85538552

@@ -8784,7 +8783,7 @@ export class Compiler extends DiagnosticEmitter {
87848783
exprs.push(
87858784
module.call(fieldInstance.internalSetterName, [
87868785
module.local_get(tempLocal.index, classTypeRef),
8787-
this.makeZero(fieldType, expression)
8786+
this.makeZero(fieldType)
87888787
], TypeRef.None)
87898788
);
87908789
this.compileFieldSetter(fieldInstance);
@@ -9080,7 +9079,7 @@ export class Compiler extends DiagnosticEmitter {
90809079
ctorInstance,
90819080
argumentExpressions,
90829081
reportNode,
9083-
this.makeZero(this.options.usizeType, reportNode),
9082+
this.makeZero(this.options.usizeType),
90849083
constraints
90859084
);
90869085
if (getExpressionType(expr) != TypeRef.None) { // possibly WILL_DROP
@@ -9647,7 +9646,7 @@ export class Compiler extends DiagnosticEmitter {
96479646
this.options.isWasm64
96489647
? BinaryOp.SubI64
96499648
: BinaryOp.SubI32,
9650-
this.makeZero(this.currentType, expression.operand),
9649+
this.makeZero(this.currentType),
96519650
expr
96529651
);
96539652
break;
@@ -10197,7 +10196,7 @@ export class Compiler extends DiagnosticEmitter {
1019710196
// === Specialized code generation ==============================================================
1019810197

1019910198
/** Makes a constant zero of the specified type. */
10200-
makeZero(type: Type, reportNode: Node): ExpressionRef {
10199+
makeZero(type: Type): ExpressionRef {
1020110200
var module = this.module;
1020210201
switch (type.kind) {
1020310202
default: assert(false);
@@ -10263,6 +10262,7 @@ export class Compiler extends DiagnosticEmitter {
1026310262
case TypeKind.U64: return module.i64(-1, -1);
1026410263
case TypeKind.F32: return module.f32(-1);
1026510264
case TypeKind.F64: return module.f64(-1);
10265+
case TypeKind.I31REF: return module.i31_new(module.i32(-1));
1026610266
}
1026710267
}
1026810268

@@ -10333,11 +10333,11 @@ export class Compiler extends DiagnosticEmitter {
1033310333
case TypeKind.EXTERNREF:
1033410334
case TypeKind.ANYREF:
1033510335
case TypeKind.EQREF:
10336-
case TypeKind.DATAREF:
10337-
case TypeKind.I31REF: {
10336+
case TypeKind.I31REF:
10337+
case TypeKind.DATAREF: {
1033810338
// Needs to be true (i.e. not zero) when the ref is _not_ null,
1033910339
// which means `ref.is_null` returns false (i.e. zero).
10340-
return module.unary(UnaryOp.EqzI32, module.ref_is(RefIsOp.RefIsNull, expr));
10340+
return module.unary(UnaryOp.EqzI32, module.ref_is_null(expr));
1034110341

1034210342
}
1034310343
default: {
@@ -10512,7 +10512,7 @@ export class Compiler extends DiagnosticEmitter {
1051210512
module.local_get(thisLocalIndex, sizeTypeRef),
1051310513
initializerNode // use initializer if present, otherwise initialize with zero
1051410514
? this.compileExpression(initializerNode, fieldType, Constraints.CONV_IMPLICIT)
10515-
: this.makeZero(fieldType, fieldPrototype.declaration)
10515+
: this.makeZero(fieldType)
1051610516
], TypeRef.None)
1051710517
);
1051810518
}
@@ -10538,7 +10538,7 @@ export class Compiler extends DiagnosticEmitter {
1053810538
if (message) {
1053910539
messageArg = this.compileExpression(message, stringInstance.type, Constraints.CONV_IMPLICIT);
1054010540
} else {
10541-
messageArg = this.makeZero(stringInstance.type, codeLocation);
10541+
messageArg = this.makeZero(stringInstance.type);
1054210542
}
1054310543

1054410544
return this.makeStaticAbort(messageArg, codeLocation);
@@ -10587,11 +10587,31 @@ export class Compiler extends DiagnosticEmitter {
1058710587
var temp = flow.getTempLocal(type);
1058810588
if (!flow.canOverflow(expr, type)) flow.setLocalFlag(temp.index, LocalFlags.WRAPPED);
1058910589
flow.setLocalFlag(temp.index, LocalFlags.NONNULL);
10590-
expr = module.if(
10591-
module.local_tee(temp.index, expr, type.isManaged),
10592-
module.local_get(temp.index, type.toRef()),
10593-
this.makeStaticAbort(this.ensureStaticString("unexpected null"), reportNode) // TODO: throw
10594-
);
10590+
10591+
var staticAbortCallExpr = this.makeStaticAbort(
10592+
this.ensureStaticString("unexpected null"),
10593+
reportNode
10594+
); // TODO: throw
10595+
10596+
if (type.isExternalReference) {
10597+
let nonNullExpr = module.local_get(temp.index, type.toRef());
10598+
if (this.options.hasFeature(Feature.GC)) {
10599+
nonNullExpr = module.ref_as_nonnull(nonNullExpr);
10600+
}
10601+
expr = module.if(
10602+
module.ref_is_null(
10603+
module.local_tee(temp.index, expr, false)
10604+
),
10605+
staticAbortCallExpr,
10606+
nonNullExpr
10607+
);
10608+
} else {
10609+
expr = module.if(
10610+
module.local_tee(temp.index, expr, type.isManaged),
10611+
module.local_get(temp.index, type.toRef()),
10612+
staticAbortCallExpr
10613+
);
10614+
}
1059510615
flow.freeTempLocal(temp);
1059610616
this.currentType = type.nonNullableType;
1059710617
return expr;
@@ -10614,6 +10634,12 @@ export class Compiler extends DiagnosticEmitter {
1061410634
var temp = flow.getTempLocal(type);
1061510635
var instanceofInstance = this.program.instanceofInstance;
1061610636
assert(this.compileFunction(instanceofInstance));
10637+
10638+
var staticAbortCallExpr = this.makeStaticAbort(
10639+
this.ensureStaticString("unexpected upcast"),
10640+
reportNode
10641+
); // TODO: throw
10642+
1061710643
if (!toType.isNullableReference || flow.isNonnull(expr, type)) {
1061810644
// Simplify if the value cannot be `null`. If toType is non-nullable, a
1061910645
// null-check would have been emitted separately so is not necessary here.
@@ -10623,7 +10649,7 @@ export class Compiler extends DiagnosticEmitter {
1062310649
module.i32(toType.classReference!.id)
1062410650
], TypeRef.I32),
1062510651
module.local_get(temp.index, type.toRef()),
10626-
this.makeStaticAbort(this.ensureStaticString("unexpected upcast"), reportNode) // TODO: throw
10652+
staticAbortCallExpr
1062710653
);
1062810654
} else {
1062910655
expr = module.if(
@@ -10634,7 +10660,7 @@ export class Compiler extends DiagnosticEmitter {
1063410660
module.i32(toType.classReference!.id)
1063510661
], TypeRef.I32),
1063610662
module.local_get(temp.index, type.toRef()),
10637-
this.makeStaticAbort(this.ensureStaticString("unexpected upcast"), reportNode) // TODO: throw
10663+
staticAbortCallExpr
1063810664
),
1063910665
module.usize(0)
1064010666
);

Diff for: ‎src/module.ts

+12
Original file line numberDiff line numberDiff line change
@@ -1774,13 +1774,25 @@ export class Module {
17741774
return binaryen._BinaryenRefIs(this.ref, op, expr);
17751775
}
17761776

1777+
ref_is_null(
1778+
expr: ExpressionRef
1779+
): ExpressionRef {
1780+
return binaryen._BinaryenRefIs(this.ref, RefIsOp.RefIsNull, expr);
1781+
}
1782+
17771783
ref_as(
17781784
op: RefAsOp,
17791785
expr: ExpressionRef
17801786
): ExpressionRef {
17811787
return binaryen._BinaryenRefAs(this.ref, op, expr);
17821788
}
17831789

1790+
ref_as_nonnull(
1791+
expr: ExpressionRef
1792+
): ExpressionRef {
1793+
return binaryen._BinaryenRefAs(this.ref, RefAsOp.RefAsNonNull, expr);
1794+
}
1795+
17841796
ref_func(
17851797
name: string,
17861798
type: TypeRef

Diff for: ‎src/passes/pass.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ import {
8181
_BinaryenMemoryFillGetValue,
8282
_BinaryenMemoryFillGetSize,
8383
_BinaryenRefIsGetValue,
84+
_BinaryenRefAsGetValue,
8485
_BinaryenTryGetBody,
8586
_BinaryenTryGetNumCatchBodies,
8687
_BinaryenTryGetCatchBodyAt,
@@ -989,7 +990,7 @@ export abstract class Visitor {
989990
}
990991
case ExpressionId.RefAs: {
991992
this.stack.push(expr);
992-
assert(false); // TODO
993+
this.visit(_BinaryenRefAsGetValue(expr));
993994
assert(this.stack.pop() == expr);
994995
this.visitRefAs(expr);
995996
break;

Diff for: ‎src/types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,7 @@ export class Type {
697697
/** 31-bit integer reference. */
698698
static readonly i31ref: Type = new Type(TypeKind.I31REF,
699699
TypeFlags.EXTERNAL |
700+
TypeFlags.NULLABLE |
700701
TypeFlags.REFERENCE, 0
701702
);
702703

0 commit comments

Comments
 (0)