Skip to content

Commit 78b3260

Browse files
authored
fix: Do proper subtype tests in instanceof (#2588)
1 parent b3cffa3 commit 78b3260

File tree

9 files changed

+4765
-889
lines changed

9 files changed

+4765
-889
lines changed

Diff for: src/bindings/js.ts

+21-21
Original file line numberDiff line numberDiff line change
@@ -950,29 +950,29 @@ export class JSBuilder extends ExportsWalker {
950950
if (type.isInternalReference) {
951951
// Lift reference types
952952
const clazz = assert(type.getClassOrWrapper(this.program));
953-
if (clazz.extends(this.program.arrayBufferInstance.prototype)) {
953+
if (clazz.extendsPrototype(this.program.arrayBufferInstance.prototype)) {
954954
sb.push("__liftBuffer(");
955955
this.needsLiftBuffer = true;
956-
} else if (clazz.extends(this.program.stringInstance.prototype)) {
956+
} else if (clazz.extendsPrototype(this.program.stringInstance.prototype)) {
957957
sb.push("__liftString(");
958958
this.needsLiftString = true;
959-
} else if (clazz.extends(this.program.arrayPrototype)) {
959+
} else if (clazz.extendsPrototype(this.program.arrayPrototype)) {
960960
let valueType = clazz.getArrayValueType();
961961
sb.push("__liftArray(");
962962
this.makeLiftFromMemory(valueType, sb);
963963
sb.push(", ");
964964
sb.push(valueType.alignLog2.toString());
965965
sb.push(", ");
966966
this.needsLiftArray = true;
967-
} else if (clazz.extends(this.program.staticArrayPrototype)) {
967+
} else if (clazz.extendsPrototype(this.program.staticArrayPrototype)) {
968968
let valueType = clazz.getArrayValueType();
969969
sb.push("__liftStaticArray(");
970970
this.makeLiftFromMemory(valueType, sb);
971971
sb.push(", ");
972972
sb.push(valueType.alignLog2.toString());
973973
sb.push(", ");
974974
this.needsLiftStaticArray = true;
975-
} else if (clazz.extends(this.program.arrayBufferViewInstance.prototype)) {
975+
} else if (clazz.extendsPrototype(this.program.arrayBufferViewInstance.prototype)) {
976976
sb.push("__liftTypedArray(");
977977
if (clazz.name == "Uint64Array") {
978978
sb.push("BigUint64Array");
@@ -1021,13 +1021,13 @@ export class JSBuilder extends ExportsWalker {
10211021
if (type.isInternalReference) {
10221022
// Lower reference types
10231023
const clazz = assert(type.getClassOrWrapper(this.program));
1024-
if (clazz.extends(this.program.arrayBufferInstance.prototype)) {
1024+
if (clazz.extendsPrototype(this.program.arrayBufferInstance.prototype)) {
10251025
sb.push("__lowerBuffer(");
10261026
this.needsLowerBuffer = true;
1027-
} else if (clazz.extends(this.program.stringInstance.prototype)) {
1027+
} else if (clazz.extendsPrototype(this.program.stringInstance.prototype)) {
10281028
sb.push("__lowerString(");
10291029
this.needsLowerString = true;
1030-
} else if (clazz.extends(this.program.arrayPrototype)) {
1030+
} else if (clazz.extendsPrototype(this.program.arrayPrototype)) {
10311031
let valueType = clazz.getArrayValueType();
10321032
sb.push("__lowerArray(");
10331033
this.makeLowerToMemory(valueType, sb);
@@ -1037,7 +1037,7 @@ export class JSBuilder extends ExportsWalker {
10371037
sb.push(clazz.getArrayValueType().alignLog2.toString());
10381038
sb.push(", ");
10391039
this.needsLowerArray = true;
1040-
} else if (clazz.extends(this.program.staticArrayPrototype)) {
1040+
} else if (clazz.extendsPrototype(this.program.staticArrayPrototype)) {
10411041
let valueType = clazz.getArrayValueType();
10421042
sb.push("__lowerStaticArray(");
10431043
this.makeLowerToMemory(valueType, sb);
@@ -1047,7 +1047,7 @@ export class JSBuilder extends ExportsWalker {
10471047
sb.push(valueType.alignLog2.toString());
10481048
sb.push(", ");
10491049
this.needsLowerStaticArray = true;
1050-
} else if (clazz.extends(this.program.arrayBufferViewInstance.prototype)) {
1050+
} else if (clazz.extendsPrototype(this.program.arrayBufferViewInstance.prototype)) {
10511051
let valueType = clazz.getArrayValueType();
10521052
sb.push("__lowerTypedArray(");
10531053
if (valueType == Type.u64) {
@@ -1079,7 +1079,7 @@ export class JSBuilder extends ExportsWalker {
10791079
this.needsLowerInternref = true;
10801080
}
10811081
sb.push(name);
1082-
if (clazz.extends(this.program.staticArrayPrototype)) {
1082+
if (clazz.extendsPrototype(this.program.staticArrayPrototype)) {
10831083
// optional last argument for __lowerStaticArray
10841084
let valueType = clazz.getArrayValueType();
10851085
if (valueType.isNumericValue) {
@@ -1397,16 +1397,16 @@ export function liftRequiresExportRuntime(type: Type): bool {
13971397
let program = clazz.program;
13981398
// flat collections lift via memory copy
13991399
if (
1400-
clazz.extends(program.arrayBufferInstance.prototype) ||
1401-
clazz.extends(program.stringInstance.prototype) ||
1402-
clazz.extends(program.arrayBufferViewInstance.prototype)
1400+
clazz.extendsPrototype(program.arrayBufferInstance.prototype) ||
1401+
clazz.extendsPrototype(program.stringInstance.prototype) ||
1402+
clazz.extendsPrototype(program.arrayBufferViewInstance.prototype)
14031403
) {
14041404
return false;
14051405
}
14061406
// nested collections lift depending on element type
14071407
if (
1408-
clazz.extends(program.arrayPrototype) ||
1409-
clazz.extends(program.staticArrayPrototype)
1408+
clazz.extendsPrototype(program.arrayPrototype) ||
1409+
clazz.extendsPrototype(program.staticArrayPrototype)
14101410
) {
14111411
return liftRequiresExportRuntime(clazz.getArrayValueType());
14121412
}
@@ -1428,11 +1428,11 @@ export function lowerRequiresExportRuntime(type: Type): bool {
14281428
// lowers using __new
14291429
let program = clazz.program;
14301430
if (
1431-
clazz.extends(program.arrayBufferInstance.prototype) ||
1432-
clazz.extends(program.stringInstance.prototype) ||
1433-
clazz.extends(program.arrayBufferViewInstance.prototype) ||
1434-
clazz.extends(program.arrayPrototype) ||
1435-
clazz.extends(program.staticArrayPrototype)
1431+
clazz.extendsPrototype(program.arrayBufferInstance.prototype) ||
1432+
clazz.extendsPrototype(program.stringInstance.prototype) ||
1433+
clazz.extendsPrototype(program.arrayBufferViewInstance.prototype) ||
1434+
clazz.extendsPrototype(program.arrayPrototype) ||
1435+
clazz.extendsPrototype(program.staticArrayPrototype)
14361436
) {
14371437
return true;
14381438
}

Diff for: src/bindings/tsd.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -249,26 +249,26 @@ export class TSDBuilder extends ExportsWalker {
249249
if (type.isInternalReference) {
250250
const sb = new Array<string>();
251251
const clazz = assert(type.getClassOrWrapper(this.program));
252-
if (clazz.extends(this.program.arrayBufferInstance.prototype)) {
252+
if (clazz.extendsPrototype(this.program.arrayBufferInstance.prototype)) {
253253
sb.push("ArrayBuffer");
254-
} else if (clazz.extends(this.program.stringInstance.prototype)) {
254+
} else if (clazz.extendsPrototype(this.program.stringInstance.prototype)) {
255255
sb.push("string");
256-
} else if (clazz.extends(this.program.arrayPrototype)) {
256+
} else if (clazz.extendsPrototype(this.program.arrayPrototype)) {
257257
const valueType = clazz.getArrayValueType();
258258
sb.push("Array<");
259259
sb.push(this.toTypeScriptType(valueType, mode));
260260
sb.push(">");
261-
} else if (clazz.extends(this.program.staticArrayPrototype)) {
261+
} else if (clazz.extendsPrototype(this.program.staticArrayPrototype)) {
262262
const valueType = clazz.getArrayValueType();
263263
sb.push("ArrayLike<");
264264
sb.push(this.toTypeScriptType(valueType, mode));
265265
sb.push(">");
266-
} else if (clazz.extends(this.program.arrayBufferViewInstance.prototype)) {
266+
} else if (clazz.extendsPrototype(this.program.arrayBufferViewInstance.prototype)) {
267267
const valueType = clazz.getArrayValueType();
268268
if (valueType == Type.i8) {
269269
sb.push("Int8Array");
270270
} else if (valueType == Type.u8) {
271-
if (clazz.extends(this.program.uint8ClampedArrayPrototype)) {
271+
if (clazz.extendsPrototype(this.program.uint8ClampedArrayPrototype)) {
272272
sb.push("Uint8ClampedArray");
273273
} else {
274274
sb.push("Uint8Array");

Diff for: src/builtins.ts

+6-6
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ function builtin_isArray(ctx: BuiltinContext): ExpressionRef {
861861
let classReference = type.getClass();
862862
return reifyConstantType(ctx,
863863
module.i32(
864-
classReference && classReference.extends(compiler.program.arrayPrototype)
864+
classReference && classReference.extendsPrototype(compiler.program.arrayPrototype)
865865
? 1
866866
: 0
867867
)
@@ -10420,26 +10420,26 @@ export function compileRTTI(compiler: Compiler): void {
1042010420
assert(instanceId == lastId++);
1042110421
let flags: TypeinfoFlags = 0;
1042210422
if (instance.isPointerfree) flags |= TypeinfoFlags.POINTERFREE;
10423-
if (instance != abvInstance && instance.extends(abvPrototype)) {
10423+
if (instance != abvInstance && instance.extendsPrototype(abvPrototype)) {
1042410424
let valueType = instance.getArrayValueType();
1042510425
flags |= TypeinfoFlags.ARRAYBUFFERVIEW;
1042610426
flags |= TypeinfoFlags.VALUE_ALIGN_0 * typeToRuntimeFlags(valueType);
10427-
} else if (instance.extends(arrayPrototype)) {
10427+
} else if (instance.extendsPrototype(arrayPrototype)) {
1042810428
let valueType = instance.getArrayValueType();
1042910429
flags |= TypeinfoFlags.ARRAY;
1043010430
flags |= TypeinfoFlags.VALUE_ALIGN_0 * typeToRuntimeFlags(valueType);
10431-
} else if (instance.extends(setPrototype)) {
10431+
} else if (instance.extendsPrototype(setPrototype)) {
1043210432
let typeArguments = assert(instance.getTypeArgumentsTo(setPrototype));
1043310433
assert(typeArguments.length == 1);
1043410434
flags |= TypeinfoFlags.SET;
1043510435
flags |= TypeinfoFlags.VALUE_ALIGN_0 * typeToRuntimeFlags(typeArguments[0]);
10436-
} else if (instance.extends(mapPrototype)) {
10436+
} else if (instance.extendsPrototype(mapPrototype)) {
1043710437
let typeArguments = assert(instance.getTypeArgumentsTo(mapPrototype));
1043810438
assert(typeArguments.length == 2);
1043910439
flags |= TypeinfoFlags.MAP;
1044010440
flags |= TypeinfoFlags.KEY_ALIGN_0 * typeToRuntimeFlags(typeArguments[0]);
1044110441
flags |= TypeinfoFlags.VALUE_ALIGN_0 * typeToRuntimeFlags(typeArguments[1]);
10442-
} else if (instance.extends(staticArrayPrototype)) {
10442+
} else if (instance.extendsPrototype(staticArrayPrototype)) {
1044310443
let valueType = instance.getArrayValueType();
1044410444
flags |= TypeinfoFlags.STATICARRAY;
1044510445
flags |= TypeinfoFlags.VALUE_ALIGN_0 * typeToRuntimeFlags(valueType);

Diff for: src/compiler.ts

+78-36
Original file line numberDiff line numberDiff line change
@@ -556,12 +556,18 @@ export class Compiler extends DiagnosticEmitter {
556556
for (let _keys = Map_keys(this.pendingInstanceOf), i = 0, k = _keys.length; i < k; ++i) {
557557
let elem = _keys[i];
558558
let name = assert(this.pendingInstanceOf.get(elem));
559-
if (elem.kind == ElementKind.Class) {
560-
this.finalizeInstanceOf(<Class>elem, name);
561-
} else if (elem.kind == ElementKind.ClassPrototype) {
562-
this.finalizeAnyInstanceOf(<ClassPrototype>elem, name);
563-
} else {
564-
assert(false);
559+
switch (elem.kind) {
560+
case ElementKind.Class:
561+
case ElementKind.Interface: {
562+
this.finalizeInstanceOf(<Class>elem, name);
563+
break;
564+
}
565+
case ElementKind.ClassPrototype:
566+
case ElementKind.InterfacePrototype: {
567+
this.finalizeAnyInstanceOf(<ClassPrototype>elem, name);
568+
break;
569+
}
570+
default: assert(false);
565571
}
566572
}
567573

@@ -6569,19 +6575,25 @@ export class Compiler extends DiagnosticEmitter {
65696575
}
65706576
let classInstance = assert(overrideInstance.getBoundClassOrInterface());
65716577
builder.addCase(classInstance.id, stmts);
6572-
// Also alias each extendee inheriting this exact overload
6573-
let extendees = classInstance.getAllExtendees(instance.declaration.name.text); // without get:/set:
6574-
for (let _values = Set_values(extendees), a = 0, b = _values.length; a < b; ++a) {
6575-
let extendee = _values[a];
6576-
builder.addCase(extendee.id, stmts);
6578+
// Also alias each extender inheriting this exact overload
6579+
let extenders = classInstance.extenders;
6580+
if (extenders) {
6581+
for (let _values = Set_values(extenders), i = 0, k = _values.length; i < k; ++i) {
6582+
let extender = _values[i];
6583+
let instanceMembers = extender.prototype.instanceMembers;
6584+
if (instanceMembers && instanceMembers.has(instance.declaration.name.text)) {
6585+
continue; // skip those not inheriting
6586+
}
6587+
builder.addCase(extender.id, stmts);
6588+
}
65776589
}
65786590
}
65796591
}
65806592

65816593
// Call the original function if no other id matches and the method is not
65826594
// abstract or part of an interface. Note that doing so will not catch an
65836595
// invalid id, but can reduce code size significantly since we also don't
6584-
// have to add branches for extendees inheriting the original function.
6596+
// have to add branches for extenders inheriting the original function.
65856597
let body: ExpressionRef;
65866598
let instanceClass = instance.getBoundClassOrInterface();
65876599
if (!instance.is(CommonFlags.Abstract) && !(instanceClass && instanceClass.kind == ElementKind.Interface)) {
@@ -7432,7 +7444,7 @@ export class Compiler extends DiagnosticEmitter {
74327444
// <nullable> instanceof <nonNullable> - LHS must be != 0
74337445
if (actualType.isNullableReference && !expectedType.isNullableReference) {
74347446

7435-
// upcast - check statically
7447+
// same or upcast - check statically
74367448
if (actualType.nonNullableType.isAssignableTo(expectedType)) {
74377449
return module.binary(
74387450
sizeTypeRef == TypeRef.I64
@@ -7443,8 +7455,8 @@ export class Compiler extends DiagnosticEmitter {
74437455
);
74447456
}
74457457

7446-
// downcast - check dynamically
7447-
if (expectedType.isAssignableTo(actualType)) {
7458+
// potential downcast - check dynamically
7459+
if (actualType.nonNullableType.hasSubtypeAssignableTo(expectedType)) {
74487460
if (!(actualType.isUnmanaged || expectedType.isUnmanaged)) {
74497461
if (this.options.pedantic) {
74507462
this.pedantic(
@@ -7477,12 +7489,13 @@ export class Compiler extends DiagnosticEmitter {
74777489
// either none or both nullable
74787490
} else {
74797491

7480-
// upcast - check statically
7492+
// same or upcast - check statically
74817493
if (actualType.isAssignableTo(expectedType)) {
74827494
return module.maybeDropCondition(expr, module.i32(1));
7495+
}
74837496

7484-
// downcast - check dynamically
7485-
} else if (expectedType.isAssignableTo(actualType)) {
7497+
// potential downcast - check dynamically
7498+
if (actualType.hasSubtypeAssignableTo(expectedType)) {
74867499
if (!(actualType.isUnmanaged || expectedType.isUnmanaged)) {
74877500
let temp = flow.getTempLocal(actualType);
74887501
let tempIndex = temp.index;
@@ -7558,19 +7571,32 @@ export class Compiler extends DiagnosticEmitter {
75587571
), false // managedness is irrelevant here, isn't interrupted
75597572
)
75607573
);
7561-
let allInstances = new Set<Class>();
7562-
allInstances.add(instance);
7563-
instance.getAllExtendeesAndImplementers(allInstances);
7564-
for (let _values = Set_values(allInstances), i = 0, k = _values.length; i < k; ++i) {
7565-
let instance = _values[i];
7566-
stmts.push(
7567-
module.br("is_instance",
7568-
module.binary(BinaryOp.EqI32,
7569-
module.local_get(1, TypeRef.I32),
7570-
module.i32(instance.id)
7574+
let allInstances: Set<Class> | null;
7575+
if (instance.isInterface) {
7576+
allInstances = instance.implementers;
7577+
} else {
7578+
allInstances = new Set();
7579+
allInstances.add(instance);
7580+
let extenders = instance.extenders;
7581+
if (extenders) {
7582+
for (let _values = Set_values(extenders), i = 0, k = _values.length; i < k; ++i) {
7583+
let extender = _values[i];
7584+
allInstances.add(extender);
7585+
}
7586+
}
7587+
}
7588+
if (allInstances) {
7589+
for (let _values = Set_values(allInstances), i = 0, k = _values.length; i < k; ++i) {
7590+
let instance = _values[i];
7591+
stmts.push(
7592+
module.br("is_instance",
7593+
module.binary(BinaryOp.EqI32,
7594+
module.local_get(1, TypeRef.I32),
7595+
module.i32(instance.id)
7596+
)
75717597
)
7572-
)
7573-
);
7598+
);
7599+
}
75747600
}
75757601
stmts.push(
75767602
module.return(
@@ -7599,7 +7625,7 @@ export class Compiler extends DiagnosticEmitter {
75997625
if (classReference) {
76007626

76017627
// static check
7602-
if (classReference.extends(prototype)) {
7628+
if (classReference.extendsPrototype(prototype)) {
76037629

76047630
// <nullable> instanceof <PROTOTYPE> - LHS must be != 0
76057631
if (actualType.isNullableReference) {
@@ -7688,8 +7714,24 @@ export class Compiler extends DiagnosticEmitter {
76887714
let allInstances = new Set<Class>();
76897715
for (let _values = Map_values(instances), i = 0, k = _values.length; i < k; ++i) {
76907716
let instance = _values[i];
7691-
allInstances.add(instance);
7692-
instance.getAllExtendeesAndImplementers(allInstances);
7717+
if (instance.isInterface) {
7718+
let implementers = instance.implementers;
7719+
if (implementers) {
7720+
for (let _values = Set_values(implementers), i = 0, k = _values.length; i < k; ++i) {
7721+
let implementer = _values[i];
7722+
allInstances.add(implementer);
7723+
}
7724+
}
7725+
} else {
7726+
allInstances.add(instance);
7727+
let extenders = instance.extenders;
7728+
if (extenders) {
7729+
for (let _values = Set_values(extenders), i = 0, k = _values.length; i < k; ++i) {
7730+
let extender = _values[i];
7731+
allInstances.add(extender);
7732+
}
7733+
}
7734+
}
76937735
}
76947736
for (let _values = Set_values(allInstances), i = 0, k = _values.length; i < k; ++i) {
76957737
let instance = _values[i];
@@ -7946,7 +7988,7 @@ export class Compiler extends DiagnosticEmitter {
79467988
let parameterTypes = instance.signature.parameterTypes;
79477989
if (parameterTypes.length) {
79487990
let first = parameterTypes[0].getClass();
7949-
if (first && !first.extends(tsaArrayInstance.prototype)) {
7991+
if (first && !first.extendsPrototype(tsaArrayInstance.prototype)) {
79507992
arrayInstance = assert(this.resolver.resolveClass(this.program.arrayPrototype, [ stringType ]));
79517993
}
79527994
}
@@ -8014,7 +8056,7 @@ export class Compiler extends DiagnosticEmitter {
80148056

80158057
// handle static arrays
80168058
let contextualClass = contextualType.getClass();
8017-
if (contextualClass && contextualClass.extends(program.staticArrayPrototype)) {
8059+
if (contextualClass && contextualClass.extendsPrototype(program.staticArrayPrototype)) {
80188060
return this.compileStaticArrayLiteral(expression, contextualType, constraints);
80198061
}
80208062

@@ -8147,7 +8189,7 @@ export class Compiler extends DiagnosticEmitter {
81478189
): ExpressionRef {
81488190
let program = this.program;
81498191
let module = this.module;
8150-
assert(!arrayInstance.extends(program.staticArrayPrototype));
8192+
assert(!arrayInstance.extendsPrototype(program.staticArrayPrototype));
81518193
let elementType = arrayInstance.getArrayValueType(); // asserts
81528194

81538195
// __newArray(length, alignLog2, classId, staticBuffer)

0 commit comments

Comments
 (0)