Skip to content

Commit 7d35a79

Browse files
errendirerrendir
committed
Avoid too eager shortcuts, handle the this type argument
1 parent a8da464 commit 7d35a79

File tree

1 file changed

+59
-50
lines changed

1 file changed

+59
-50
lines changed

‎src/compiler/checker.ts

Lines changed: 59 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ namespace ts {
8585
/** This will be set during calls to `getResolvedSignature` where services determines an apparent number of arguments greater than what is actually provided. */
8686
let apparentArgumentCount: number | undefined;
8787

88+
let maybeExtensionRelationKey: string | null = null;
89+
8890
// for public members that accept a Node or one of its subtypes, we must guard against
8991
// synthetic nodes created during transformations by calling `getParseTreeNode`.
9092
// for most of these, we perform the guard only on `checker` to avoid any possible
@@ -9408,7 +9410,7 @@ namespace ts {
94089410
// If source and target are already being compared, consider them related with assumptions
94099411
if (id === maybeKeys[i]) {
94109412
if(compilerOptions.logCheckerShortcut) {
9411-
perfLog("Relating", typeToString(source), 'to', typeToString(target), 'finished due to cycle danger')
9413+
//perfLog("Relating", typeToString(source), 'to', typeToString(target), 'finished due to cycle danger')
94129414
}
94139415
return Ternary.Maybe;
94149416
}
@@ -9582,13 +9584,10 @@ namespace ts {
95829584
// we can avoid potentially very expensive resursion through the structure of A and B with different
95839585
// type arguments
95849586

9585-
// @ts-ignore
9586-
const sourceConcrete = <TypeReference>source
9587-
const sourceGeneric = (<TypeReference>source).target
95889587
const isSourceInterfaceOrClass = getObjectFlags((<TypeReference>source).target) & ObjectFlags.ClassOrInterface
9589-
if(isSourceInterfaceOrClass && !!sourceGeneric.typeParameters && !!sourceConcrete.typeArguments && sourceGeneric.typeParameters.length === sourceConcrete.typeArguments.length) {
9590-
let sourceBaseTypeCompatibleWithTarget: TypeReference | null = null
9591-
let sourceAsInherited: TypeReference | null = null
9588+
// TODO: Remove the second condition
9589+
if(isSourceInterfaceOrClass && compilerOptions.logCheckerShortcut) {
9590+
let sourceAsBaseType: TypeReference | null = null
95929591

95939592
// TODO: Make this check cachable.
95949593
// TODO: Verify that the base type recursion works correctly
@@ -9597,41 +9596,68 @@ namespace ts {
95979596
const referenceBaseTypes = getBaseTypes(lookupType.target).filter(
95989597
baseType => (getObjectFlags(baseType) & ObjectFlags.Reference)
95999598
) as TypeReference[]
9600-
compilerOptions.logCheckerShortcut &&
9601-
perfLog(typeToString(lookupType.target), "inherits from", referenceBaseTypes.map(type => typeToString(type)).join(","))
9599+
//perfLog(typeToString(lookupType.target), "inherits from", referenceBaseTypes.map(type => typeToString(type)).join(","))
9600+
9601+
const needsToBeMapped = lookupType.typeArguments !== undefined
9602+
let thisType: Type
9603+
let typeMapper: TypeMapper
9604+
if(needsToBeMapped) {
9605+
const typeParameters = lookupType.target.typeParameters || emptyArray
9606+
let typeArguments = lookupType.typeArguments
9607+
// Remove "this" type argument
9608+
if(typeArguments.length === typeParameters.length + 1) {
9609+
thisType = typeArguments[typeArguments.length-1]
9610+
typeArguments = typeArguments.slice(0,-1)
9611+
}
9612+
typeMapper = createTypeMapper(typeParameters, typeArguments)
9613+
}
9614+
const stepDownIntoBaseType = (baseType: TypeReference): TypeReference => {
9615+
let sourceAsBaseType: TypeReference = baseType
9616+
sourceAsBaseType = (needsToBeMapped ? instantiateType(sourceAsBaseType, typeMapper) : sourceAsBaseType) as TypeReference
9617+
sourceAsBaseType = (thisType ? getTypeWithThisArgument(sourceAsBaseType, thisType) : sourceAsBaseType) as TypeReference
9618+
9619+
// TODO: Is there a method for forcibly replacing the this type? Why is it even necessary?
9620+
if(thisType) {
9621+
sourceAsBaseType = createTypeReference(sourceAsBaseType.target,
9622+
concatenate(sourceAsBaseType.typeArguments.slice(0, sourceAsBaseType.target.typeParameters.length), [thisType])
9623+
)
9624+
}
9625+
return sourceAsBaseType as TypeReference
9626+
}
96029627

96039628
for(const baseType of referenceBaseTypes) {
96049629
if(baseType.target === targetGeneric) {
9605-
sourceBaseTypeCompatibleWithTarget = baseType
9606-
const typeMapper = createTypeMapper(lookupType.target.typeParameters, lookupType.typeArguments)
9607-
// instantiateType seems to actually be of type <T extends Type>(type: T, mapper: TypeMapper) => T
9608-
sourceAsInherited = <TypeReference>instantiateType(baseType, typeMapper)
9630+
// Both `getTypeWithThisArgument` and `instantiateType` seem to actually
9631+
// be of type <T extends Type>(type: T, mapper: TypeMapper) => T
9632+
sourceAsBaseType = stepDownIntoBaseType(baseType)
9633+
96099634
return
96109635
}
96119636
}
96129637

96139638
// Attempt recursive check if needed
96149639
for(const baseType of referenceBaseTypes) {
9615-
const typeMapper = createTypeMapper(lookupType.target.typeParameters, lookupType.typeArguments)
9616-
const sourceAsInherited = <TypeReference>instantiateType(baseType, typeMapper)
9617-
9618-
findBaseTargetType(sourceAsInherited, targetGeneric)
9640+
const sourceAsBaseType = stepDownIntoBaseType(baseType)
9641+
findBaseTargetType(sourceAsBaseType, targetGeneric)
96199642
}
96209643
}
96219644

96229645
findBaseTargetType(<TypeReference>source, (<TypeReference>target).target)
9623-
9624-
if(sourceBaseTypeCompatibleWithTarget !== null) {
9625-
compilerOptions.logCheckerShortcut &&
9626-
perfLog("A shortcut is availalbe:", typeToString(source), "-(known)->", typeToString(sourceAsInherited), "->", typeToString(target))
9627-
9628-
const result = isRelatedTo(sourceAsInherited, target, false)
9629-
compilerOptions.logCheckerShortcut &&
9630-
perfLog("\tconcretes verified", result === Ternary.Maybe ? "maybe" : result === Ternary.True ? "true" : result === Ternary.False ? "false" : "waat?")
9631-
if(result) {
9632-
compilerOptions.logCheckerShortcut &&
9633-
perfLog("Shortcut was successfully taken")
9634-
return result
9646+
9647+
//const ternaryToString = (result: Ternary) => result === Ternary.Maybe ? "maybe" : result === Ternary.True ? "true" : result === Ternary.False ? "false" : "waat?"
9648+
9649+
if(sourceAsBaseType !== null) {
9650+
const shortcutRelationKey = getRelationKey(source, sourceAsBaseType, relation)
9651+
const currentlyVerifyingShortcut = shortcutRelationKey === maybeExtensionRelationKey
9652+
9653+
//perfLog("A shortcut is availalbe:", typeToString(source), `-(${shortcutRelationKey};${maybeExtensionRelationKey};${currentlyVerifyingShortcut})->`, typeToString(sourceAsBaseType), "->", typeToString(target))
9654+
9655+
if(!currentlyVerifyingShortcut) {
9656+
result = isRelatedTo(sourceAsBaseType, target, false)
9657+
if(result) {
9658+
//perfLog("Shortcut was successfully taken")
9659+
return result
9660+
}
96359661
}
96369662
}
96379663
}
@@ -9657,30 +9683,19 @@ namespace ts {
96579683
result = isGenericMappedType(source) ? mappedTypeRelatedTo(<MappedType>source, <MappedType>target, reportStructuralErrors) : Ternary.False;
96589684
}
96599685
else {
9660-
const times = [Date.now()]
96619686
result = propertiesRelatedTo(source, target, reportStructuralErrors);
9662-
times.push(Date.now())
96639687
if (result) {
96649688
result &= signaturesRelatedTo(source, target, SignatureKind.Call, reportStructuralErrors);
9665-
times.push(Date.now())
96669689
if (result) {
96679690
result &= signaturesRelatedTo(source, target, SignatureKind.Construct, reportStructuralErrors);
9668-
times.push(Date.now())
96699691
if (result) {
96709692
result &= indexTypesRelatedTo(source, target, IndexKind.String, sourceIsPrimitive, reportStructuralErrors);
9671-
times.push(Date.now())
96729693
if (result) {
96739694
result &= indexTypesRelatedTo(source, target, IndexKind.Number, sourceIsPrimitive, reportStructuralErrors);
9674-
times.push(Date.now())
96759695
}
96769696
}
96779697
}
96789698
}
9679-
times.push(Date.now())
9680-
const elapsedTime = times[times.length-1] - times[0]
9681-
if(compilerOptions.logLongCheckerCalls && elapsedTime > (compilerOptions.logLongTimeThreshold || 100)) {
9682-
perfLog(`Long structural check (${elapsedTime}ms - ${times.slice(1).map((time,i) => time-times[i]).join('-')})`, typeToString(source), typeToString(target))
9683-
}
96849699
}
96859700
if (result) {
96869701
if (!originalErrorInfo) {
@@ -9782,16 +9797,7 @@ namespace ts {
97829797
}
97839798
return Ternary.False;
97849799
}
9785-
const startTime = Date.now()
9786-
if(compilerOptions.logCheckerShortcut) {
9787-
perfLog("Started property relation")
9788-
}
97899800
const related = isRelatedTo(getTypeOfSymbol(sourceProp), getTypeOfSymbol(targetProp), reportErrors);
9790-
if(compilerOptions.logCheckerShortcut) {
9791-
const elapsedTime = Date.now() - startTime
9792-
perfLog(`Finished property relation in ${elapsedTime}ms`, typeToString(getTypeOfSymbol(sourceProp)), typeToString(getTypeOfSymbol(targetProp)))
9793-
perfLog(symbolToString(sourceProp), symbolToString(targetProp))
9794-
}
97959801
if (!related) {
97969802
if (reportErrors) {
97979803
reportError(Diagnostics.Types_of_property_0_are_incompatible, symbolToString(targetProp));
@@ -22340,7 +22346,10 @@ namespace ts {
2234022346
if (checkInheritedPropertiesAreIdentical(type, node.name)) {
2234122347
for (const baseType of getBaseTypes(type)) {
2234222348
const startTime = Date.now()
22343-
checkTypeAssignableTo(typeWithThis, getTypeWithThisArgument(baseType, type.thisType), node.name, Diagnostics.Interface_0_incorrectly_extends_interface_1);
22349+
const target = getTypeWithThisArgument(baseType, type.thisType);
22350+
maybeExtensionRelationKey = getRelationKey(typeWithThis, target, assignableRelation);
22351+
checkTypeAssignableTo(typeWithThis, target, node.name, Diagnostics.Interface_0_incorrectly_extends_interface_1);
22352+
maybeExtensionRelationKey = null
2234422353
const elapsedTime = Date.now() - startTime
2234522354
if(compilerOptions.logLongCheckerCalls && elapsedTime > (compilerOptions.logLongTimeThreshold || 100)) {
2234622355
perfLog(`Long inheritance check (${elapsedTime}ms) of`, typeToString(type), "against", typeToString(baseType))

0 commit comments

Comments
 (0)