Skip to content

Commit

Permalink
Persist inference context object throughout the signature, and add is…
Browse files Browse the repository at this point in the history
…Fixed property
  • Loading branch information
JsonFreeman committed Mar 14, 2015
1 parent f6de919 commit df6f856
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 20 deletions.
46 changes: 26 additions & 20 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3505,6 +3505,7 @@ module ts {
return t => {
for (let i = 0; i < context.typeParameters.length; i++) {
if (t === context.typeParameters[i]) {
context.inferences[i].isFixed = true;
return getInferredType(context, i);
}
}
Expand Down Expand Up @@ -4563,12 +4564,11 @@ module ts {
function createInferenceContext(typeParameters: TypeParameter[], inferUnionTypes: boolean): InferenceContext {
let inferences: TypeInferences[] = [];
for (let unused of typeParameters) {
inferences.push({ primary: undefined, secondary: undefined });
inferences.push({ primary: undefined, secondary: undefined, isFixed: false });
}
return {
typeParameters: typeParameters,

This comment has been minimized.

Copy link
@yuit

yuit Mar 17, 2015

Contributor

shorthand property assignment

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 17, 2015

Author Contributor

Yes, good idea.

inferUnionTypes: inferUnionTypes,
inferenceCount: 0,
inferences: inferences,
inferredTypes: new Array(typeParameters.length),
};
Expand Down Expand Up @@ -4615,11 +4615,13 @@ module ts {
for (let i = 0; i < typeParameters.length; i++) {
if (target === typeParameters[i]) {
let inferences = context.inferences[i];
let candidates = inferiority ?
inferences.secondary || (inferences.secondary = []) :
inferences.primary || (inferences.primary = []);
if (!contains(candidates, source)) candidates.push(source);
break;
if (!inferences.isFixed) {
let candidates = inferiority ?
inferences.secondary || (inferences.secondary = []) :
inferences.primary || (inferences.primary = []);
if (!contains(candidates, source)) candidates.push(source);
}
return;
}
}
}
Expand Down Expand Up @@ -6336,11 +6338,15 @@ module ts {
return getSignatureInstantiation(signature, getInferredTypes(context));
}

function inferTypeArguments(signature: Signature, args: Expression[], excludeArgument: boolean[]): InferenceContext {
function inferTypeArguments(signature: Signature, args: Expression[], excludeArgument: boolean[], context: InferenceContext): void {
let typeParameters = signature.typeParameters;
let context = createInferenceContext(typeParameters, /*inferUnionTypes*/ false);
let inferenceMapper = createInferenceMapper(context);

// Clear out all the inference results from the last time inferTypeArguments was called on this context

This comment has been minimized.

Copy link
@yuit

yuit Mar 17, 2015

Contributor

Why do we have to do this? and previously we don't. It seems to me that we pass in similar context from outside except that it can be undefined

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 17, 2015

Author Contributor

When we used to createInferenceContext in this function, we initialized it with an empty array for inferredTypes. So every call to inferTypeArguments had a fresh array. Notice now that we call this inside the while (true) loop in chooseOverload, but we createInferenceContext outside the loop. So on the first iteration, the call to inferTypeArguments won't need to clear out the type arguments, but on all subsequent iterations, it will have the values from the previous iteration. I could also move this clearing logic into chooseOverload toward the bottom of the loop.

for (let i = 0; i < typeParameters.length; i++) {
context.inferredTypes[i] = undefined;
}

// We perform two passes over the arguments. In the first pass we infer from all arguments, but use
// wildcards for all context sensitive function expressions.
for (let i = 0; i < args.length; i++) {
Expand Down Expand Up @@ -6385,8 +6391,6 @@ module ts {
inferredTypes[i] = unknownType;
}
}

return context;
}

function checkTypeArguments(signature: Signature, typeArguments: TypeNode[], typeArgumentResultTypes: Type[], reportErrors: boolean): boolean {
Expand Down Expand Up @@ -6620,15 +6624,17 @@ module ts {
return resolveErrorCall(node);

function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>) {
for (let current of candidates) {
if (!hasCorrectArity(node, args, current)) {
for (let originalCandidate of candidates) {
if (!hasCorrectArity(node, args, originalCandidate)) {
continue;
}

let originalCandidate = current;
let inferenceResult: InferenceContext;

let candidate: Signature;
let typeArgumentsAreValid: boolean;
let inferenceContext = originalCandidate.typeParameters
? createInferenceContext(originalCandidate.typeParameters, /*inferUnionTypes*/ false)
: undefined;

while (true) {
candidate = originalCandidate;
if (candidate.typeParameters) {
Expand All @@ -6638,9 +6644,9 @@ module ts {
typeArgumentsAreValid = checkTypeArguments(candidate, typeArguments, typeArgumentTypes, /*reportErrors*/ false)
}
else {
inferenceResult = inferTypeArguments(candidate, args, excludeArgument);
typeArgumentsAreValid = inferenceResult.failedTypeParameterIndex < 0;
typeArgumentTypes = inferenceResult.inferredTypes;
inferTypeArguments(candidate, args, excludeArgument, inferenceContext);

This comment has been minimized.

Copy link
@yuit

yuit Mar 17, 2015

Contributor

can the inferenceContext be undefined here ? if so in the inferTypeArguments, shouldn't we guard against it?

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 17, 2015

Author Contributor

It cannot be undefined. It's only undefined when originalCandidate.typeParameters is undefined. But here we are guarded by if (candidate.typeParameters), and notice from the line just above that, candidate = originalCandidate

typeArgumentsAreValid = inferenceContext.failedTypeParameterIndex < 0;
typeArgumentTypes = inferenceContext.inferredTypes;
}
if (!typeArgumentsAreValid) {
break;
Expand Down Expand Up @@ -6670,7 +6676,7 @@ module ts {
else {
candidateForTypeArgumentError = originalCandidate;
if (!typeArguments) {
resultOfFailedInference = inferenceResult;
resultOfFailedInference = inferenceContext;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,8 @@ module ts {
export interface TypeInferences {
primary: Type[]; // Inferences made directly to a type parameter
secondary: Type[]; // Inferences made to a type parameter in a union type
isFixed: boolean; // Whether the type parameter is fixed, as defined in section 4.12.2 of the TypeScript spec
// If a type parameter is fixed, no more inferences can be made for the type parameter
}

export interface InferenceContext {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/APISample_compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,7 @@ declare module "typescript" {
interface TypeInferences {
primary: Type[];
secondary: Type[];
isFixed: boolean;
}
interface InferenceContext {
typeParameters: TypeParameter[];
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/APISample_compile.types
Original file line number Diff line number Diff line change
Expand Up @@ -3767,6 +3767,9 @@ declare module "typescript" {
secondary: Type[];
>secondary : Type[]
>Type : Type

isFixed: boolean;
>isFixed : boolean
}
interface InferenceContext {
>InferenceContext : InferenceContext
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/APISample_linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ declare module "typescript" {
interface TypeInferences {
primary: Type[];
secondary: Type[];
isFixed: boolean;
}
interface InferenceContext {
typeParameters: TypeParameter[];
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/APISample_linter.types
Original file line number Diff line number Diff line change
Expand Up @@ -3913,6 +3913,9 @@ declare module "typescript" {
secondary: Type[];
>secondary : Type[]
>Type : Type

isFixed: boolean;
>isFixed : boolean
}
interface InferenceContext {
>InferenceContext : InferenceContext
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/APISample_transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ declare module "typescript" {
interface TypeInferences {
primary: Type[];
secondary: Type[];
isFixed: boolean;
}
interface InferenceContext {
typeParameters: TypeParameter[];
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/APISample_transform.types
Original file line number Diff line number Diff line change
Expand Up @@ -3863,6 +3863,9 @@ declare module "typescript" {
secondary: Type[];
>secondary : Type[]
>Type : Type

isFixed: boolean;
>isFixed : boolean
}
interface InferenceContext {
>InferenceContext : InferenceContext
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/APISample_watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,7 @@ declare module "typescript" {
interface TypeInferences {
primary: Type[];
secondary: Type[];
isFixed: boolean;
}
interface InferenceContext {
typeParameters: TypeParameter[];
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/APISample_watcher.types
Original file line number Diff line number Diff line change
Expand Up @@ -4036,6 +4036,9 @@ declare module "typescript" {
secondary: Type[];
>secondary : Type[]
>Type : Type

isFixed: boolean;
>isFixed : boolean
}
interface InferenceContext {
>InferenceContext : InferenceContext
Expand Down

0 comments on commit df6f856

Please sign in to comment.