Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter criteria for eliminating types in unions during inference #32919

Merged
merged 8 commits into from
Aug 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 23 additions & 27 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15486,8 +15486,7 @@ namespace ts {
let visited: Map<number>;
let bivariant = false;
let propagationType: Type;
let inferenceMatch = false;
let inferenceIncomplete = false;
let inferencePriority = InferencePriority.MaxValue;
let allowComplexConstraintInference = true;
inferFromTypes(originalSource, originalTarget);

Expand Down Expand Up @@ -15600,7 +15599,7 @@ namespace ts {
clearCachedInferences(inferences);
}
}
inferenceMatch = true;
inferencePriority = Math.min(inferencePriority, priority);
return;
}
else {
Expand Down Expand Up @@ -15694,19 +15693,15 @@ namespace ts {
const key = source.id + "," + target.id;
const status = visited && visited.get(key);
if (status !== undefined) {
if (status & 1) inferenceMatch = true;
if (status & 2) inferenceIncomplete = true;
inferencePriority = Math.min(inferencePriority, status);
return;
}
(visited || (visited = createMap<number>())).set(key, 0);
const saveInferenceMatch = inferenceMatch;
const saveInferenceIncomplete = inferenceIncomplete;
inferenceMatch = false;
inferenceIncomplete = false;
(visited || (visited = createMap<number>())).set(key, InferencePriority.Circularity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the -1 here (used as number) the same -1 lower in the file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

const saveInferencePriority = inferencePriority;
inferencePriority = InferencePriority.MaxValue;
action(source, target);
visited.set(key, (inferenceMatch ? 1 : 0) | (inferenceIncomplete ? 2 : 0));
inferenceMatch = inferenceMatch || saveInferenceMatch;
inferenceIncomplete = inferenceIncomplete || saveInferenceIncomplete;
visited.set(key, inferencePriority);
inferencePriority = Math.min(inferencePriority, saveInferencePriority);
}

function inferFromMatchingType(source: Type, targets: Type[], matches: (s: Type, t: Type) => boolean) {
Expand Down Expand Up @@ -15778,31 +15773,32 @@ namespace ts {
let nakedTypeVariable: Type | undefined;
const sources = source.flags & TypeFlags.Union ? (<UnionType>source).types : [source];
const matched = new Array<boolean>(sources.length);
const saveInferenceIncomplete = inferenceIncomplete;
inferenceIncomplete = false;
let inferenceCircularity = false;
// First infer to types that are not naked type variables. For each source type we
// track whether inferences were made from that particular type to some target.
// track whether inferences were made from that particular type to some target with
// equal priority (i.e. of equal quality) to what we would infer for a naked type
// parameter.
for (const t of targets) {
if (getInferenceInfoForType(t)) {
nakedTypeVariable = t;
typeVariableCount++;
}
else {
for (let i = 0; i < sources.length; i++) {
const saveInferenceMatch = inferenceMatch;
inferenceMatch = false;
const saveInferencePriority = inferencePriority;
inferencePriority = InferencePriority.MaxValue;
inferFromTypes(sources[i], t);
if (inferenceMatch) matched[i] = true;
inferenceMatch = inferenceMatch || saveInferenceMatch;
if (inferencePriority === priority) matched[i] = true;
inferenceCircularity = inferenceCircularity || inferencePriority === InferencePriority.Circularity;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for inferencePriority to be a negative value that isn't -1 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No

inferencePriority = Math.min(inferencePriority, saveInferencePriority);
}
}
}
const inferenceComplete = !inferenceIncomplete;
inferenceIncomplete = inferenceIncomplete || saveInferenceIncomplete;
// If the target has a single naked type variable and inference completed (meaning we
// explored the types fully), create a union of the source types from which no inferences
// have been made so far and infer from that union to the naked type variable.
if (typeVariableCount === 1 && inferenceComplete) {
// If the target has a single naked type variable and no inference circularities were
// encountered above (meaning we explored the types fully), create a union of the source
// types from which no inferences have been made so far and infer from that union to the
// naked type variable.
if (typeVariableCount === 1 && !inferenceCircularity) {
const unmatched = flatMap(sources, (s, i) => matched[i] ? undefined : s);
if (unmatched.length) {
inferFromTypes(getUnionType(unmatched), nakedTypeVariable!);
Expand Down Expand Up @@ -15905,7 +15901,7 @@ namespace ts {
const symbol = isNonConstructorObject ? target.symbol : undefined;
if (symbol) {
if (contains(symbolStack, symbol)) {
inferenceIncomplete = true;
inferencePriority = InferencePriority.Circularity;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't tell if -1 is being used here because it's lower than anything else, has all bits set, or both. A named enum member with the intended semantics of this would be clearer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because -1 is less than any normal priority value. I'll give it a symbolic name.

return;
}
(symbolStack || (symbolStack = [])).push(symbol);
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4471,8 +4471,10 @@ namespace ts {
LiteralKeyof = 1 << 5, // Inference made from a string literal to a keyof T
NoConstraints = 1 << 6, // Don't infer from constraints of instantiable types
AlwaysStrict = 1 << 7, // Always use strict rules for contravariant inferences
MaxValue = 1 << 8, // Seed for inference priority tracking

PriorityImpliesCombination = ReturnType | MappedTypeConstraint | LiteralKeyof, // These priorities imply that the resulting type should be a combination of all candidates
Circularity = -1, // Inference circularity (value less than all other priorities)
}

/* @internal */
Expand Down
4 changes: 3 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2438,7 +2438,9 @@ declare namespace ts {
LiteralKeyof = 32,
NoConstraints = 64,
AlwaysStrict = 128,
PriorityImpliesCombination = 56
MaxValue = 256,
PriorityImpliesCombination = 56,
Circularity = -1
}
/** @deprecated Use FileExtensionInfo instead. */
export type JsFileExtensionInfo = FileExtensionInfo;
Expand Down
4 changes: 3 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2438,7 +2438,9 @@ declare namespace ts {
LiteralKeyof = 32,
NoConstraints = 64,
AlwaysStrict = 128,
PriorityImpliesCombination = 56
MaxValue = 256,
PriorityImpliesCombination = 56,
Circularity = -1
}
/** @deprecated Use FileExtensionInfo instead. */
export type JsFileExtensionInfo = FileExtensionInfo;
Expand Down
26 changes: 26 additions & 0 deletions tests/baselines/reference/unionTypeInference.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,30 @@ tests/cases/conformance/types/typeRelationships/typeInference/unionTypeInference

declare function bar<T>(x: T, y: string | T): T;
const y = bar(1, 2);

// Repro from #32752

const containsPromises: unique symbol = Symbol();

type DeepPromised<T> =
{ [containsPromises]?: true } &
{ [TKey in keyof T]: T[TKey] | DeepPromised<T[TKey]> | Promise<DeepPromised<T[TKey]>> };

async function fun<T>(deepPromised: DeepPromised<T>) {
const deepPromisedWithIndexer: DeepPromised<{ [name: string]: {} | null | undefined }> = deepPromised;
for (const value of Object.values(deepPromisedWithIndexer)) {
const awaitedValue = await value;
if (awaitedValue)
await fun(awaitedValue);
}
}

// Repro from #32752

type Deep<T> = { [K in keyof T]: T[K] | Deep<T[K]> };

declare function baz<T>(dp: Deep<T>): T;
declare let xx: { a: string | undefined };

baz(xx);

71 changes: 53 additions & 18 deletions tests/baselines/reference/unionTypeInference.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,64 @@ foo(x);

declare function bar<T>(x: T, y: string | T): T;
const y = bar(1, 2);

// Repro from #32752

const containsPromises: unique symbol = Symbol();

type DeepPromised<T> =
{ [containsPromises]?: true } &
{ [TKey in keyof T]: T[TKey] | DeepPromised<T[TKey]> | Promise<DeepPromised<T[TKey]>> };

async function fun<T>(deepPromised: DeepPromised<T>) {
const deepPromisedWithIndexer: DeepPromised<{ [name: string]: {} | null | undefined }> = deepPromised;
for (const value of Object.values(deepPromisedWithIndexer)) {
const awaitedValue = await value;
if (awaitedValue)
await fun(awaitedValue);
}
}

// Repro from #32752

type Deep<T> = { [K in keyof T]: T[K] | Deep<T[K]> };

declare function baz<T>(dp: Deep<T>): T;
declare let xx: { a: string | undefined };

baz(xx);


//// [unionTypeInference.js]
"use strict";
exports.__esModule = true;
var a1 = f1(1, 2); // 1 | 2
var a2 = f1(1, "hello"); // 1
var a3 = f1(1, sn); // number
var a4 = f1(undefined, "abc"); // undefined
var a5 = f1("foo", "bar"); // "foo"
var a6 = f1(true, false); // boolean
var a7 = f1("hello", 1); // Error
const a1 = f1(1, 2); // 1 | 2
const a2 = f1(1, "hello"); // 1
const a3 = f1(1, sn); // number
const a4 = f1(undefined, "abc"); // undefined
const a5 = f1("foo", "bar"); // "foo"
const a6 = f1(true, false); // boolean
const a7 = f1("hello", 1); // Error
var b1 = f2(["string", true]); // boolean
var c1 = f3(5); // 5
var c2 = f3(sn); // number
var c3 = f3(true); // true
var c4 = f3(b); // true
var c5 = f3("abc"); // never
var d1 = f4("abc");
var d2 = f4(s);
var d3 = f4(42); // Error
const c1 = f3(5); // 5
const c2 = f3(sn); // number
const c3 = f3(true); // true
const c4 = f3(b); // true
const c5 = f3("abc"); // never
const d1 = f4("abc");
const d2 = f4(s);
const d3 = f4(42); // Error
function qux(p1, p2) {
p1 = p2;
}
foo(x);
var y = bar(1, 2);
const y = bar(1, 2);
// Repro from #32752
const containsPromises = Symbol();
async function fun(deepPromised) {
const deepPromisedWithIndexer = deepPromised;
for (const value of Object.values(deepPromisedWithIndexer)) {
const awaitedValue = await value;
if (awaitedValue)
await fun(awaitedValue);
}
}
baz(xx);
93 changes: 91 additions & 2 deletions tests/baselines/reference/unionTypeInference.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ declare function foo<T>(x: T | Promise<T>): void;
>T : Symbol(T, Decl(unionTypeInference.ts, 45, 21))
>x : Symbol(x, Decl(unionTypeInference.ts, 45, 24))
>T : Symbol(T, Decl(unionTypeInference.ts, 45, 21))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
>T : Symbol(T, Decl(unionTypeInference.ts, 45, 21))

declare let x: false | Promise<true>;
>x : Symbol(x, Decl(unionTypeInference.ts, 46, 11))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))

foo(x);
>foo : Symbol(foo, Decl(unionTypeInference.ts, 41, 1))
Expand All @@ -187,3 +187,92 @@ const y = bar(1, 2);
>y : Symbol(y, Decl(unionTypeInference.ts, 50, 5))
>bar : Symbol(bar, Decl(unionTypeInference.ts, 47, 7))

// Repro from #32752

const containsPromises: unique symbol = Symbol();
>containsPromises : Symbol(containsPromises, Decl(unionTypeInference.ts, 54, 5))
>Symbol : Symbol(Symbol, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.symbol.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2019.symbol.d.ts, --, --))

type DeepPromised<T> =
>DeepPromised : Symbol(DeepPromised, Decl(unionTypeInference.ts, 54, 49))
>T : Symbol(T, Decl(unionTypeInference.ts, 56, 18))

{ [containsPromises]?: true } &
>[containsPromises] : Symbol([containsPromises], Decl(unionTypeInference.ts, 57, 5))
>containsPromises : Symbol(containsPromises, Decl(unionTypeInference.ts, 54, 5))

{ [TKey in keyof T]: T[TKey] | DeepPromised<T[TKey]> | Promise<DeepPromised<T[TKey]>> };
>TKey : Symbol(TKey, Decl(unionTypeInference.ts, 58, 7))
>T : Symbol(T, Decl(unionTypeInference.ts, 56, 18))
>T : Symbol(T, Decl(unionTypeInference.ts, 56, 18))
>TKey : Symbol(TKey, Decl(unionTypeInference.ts, 58, 7))
>DeepPromised : Symbol(DeepPromised, Decl(unionTypeInference.ts, 54, 49))
>T : Symbol(T, Decl(unionTypeInference.ts, 56, 18))
>TKey : Symbol(TKey, Decl(unionTypeInference.ts, 58, 7))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
>DeepPromised : Symbol(DeepPromised, Decl(unionTypeInference.ts, 54, 49))
>T : Symbol(T, Decl(unionTypeInference.ts, 56, 18))
>TKey : Symbol(TKey, Decl(unionTypeInference.ts, 58, 7))

async function fun<T>(deepPromised: DeepPromised<T>) {
>fun : Symbol(fun, Decl(unionTypeInference.ts, 58, 92))
>T : Symbol(T, Decl(unionTypeInference.ts, 60, 19))
>deepPromised : Symbol(deepPromised, Decl(unionTypeInference.ts, 60, 22))
>DeepPromised : Symbol(DeepPromised, Decl(unionTypeInference.ts, 54, 49))
>T : Symbol(T, Decl(unionTypeInference.ts, 60, 19))

const deepPromisedWithIndexer: DeepPromised<{ [name: string]: {} | null | undefined }> = deepPromised;
>deepPromisedWithIndexer : Symbol(deepPromisedWithIndexer, Decl(unionTypeInference.ts, 61, 9))
>DeepPromised : Symbol(DeepPromised, Decl(unionTypeInference.ts, 54, 49))
>name : Symbol(name, Decl(unionTypeInference.ts, 61, 51))
>deepPromised : Symbol(deepPromised, Decl(unionTypeInference.ts, 60, 22))

for (const value of Object.values(deepPromisedWithIndexer)) {
>value : Symbol(value, Decl(unionTypeInference.ts, 62, 14))
>Object.values : Symbol(ObjectConstructor.values, Decl(lib.es2017.object.d.ts, --, --), Decl(lib.es2017.object.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>values : Symbol(ObjectConstructor.values, Decl(lib.es2017.object.d.ts, --, --), Decl(lib.es2017.object.d.ts, --, --))
>deepPromisedWithIndexer : Symbol(deepPromisedWithIndexer, Decl(unionTypeInference.ts, 61, 9))

const awaitedValue = await value;
>awaitedValue : Symbol(awaitedValue, Decl(unionTypeInference.ts, 63, 13))
>value : Symbol(value, Decl(unionTypeInference.ts, 62, 14))

if (awaitedValue)
>awaitedValue : Symbol(awaitedValue, Decl(unionTypeInference.ts, 63, 13))

await fun(awaitedValue);
>fun : Symbol(fun, Decl(unionTypeInference.ts, 58, 92))
>awaitedValue : Symbol(awaitedValue, Decl(unionTypeInference.ts, 63, 13))
}
}

// Repro from #32752

type Deep<T> = { [K in keyof T]: T[K] | Deep<T[K]> };
>Deep : Symbol(Deep, Decl(unionTypeInference.ts, 67, 1))
>T : Symbol(T, Decl(unionTypeInference.ts, 71, 10))
>K : Symbol(K, Decl(unionTypeInference.ts, 71, 18))
>T : Symbol(T, Decl(unionTypeInference.ts, 71, 10))
>T : Symbol(T, Decl(unionTypeInference.ts, 71, 10))
>K : Symbol(K, Decl(unionTypeInference.ts, 71, 18))
>Deep : Symbol(Deep, Decl(unionTypeInference.ts, 67, 1))
>T : Symbol(T, Decl(unionTypeInference.ts, 71, 10))
>K : Symbol(K, Decl(unionTypeInference.ts, 71, 18))

declare function baz<T>(dp: Deep<T>): T;
>baz : Symbol(baz, Decl(unionTypeInference.ts, 71, 53))
>T : Symbol(T, Decl(unionTypeInference.ts, 73, 21))
>dp : Symbol(dp, Decl(unionTypeInference.ts, 73, 24))
>Deep : Symbol(Deep, Decl(unionTypeInference.ts, 67, 1))
>T : Symbol(T, Decl(unionTypeInference.ts, 73, 21))
>T : Symbol(T, Decl(unionTypeInference.ts, 73, 21))

declare let xx: { a: string | undefined };
>xx : Symbol(xx, Decl(unionTypeInference.ts, 74, 11))
>a : Symbol(a, Decl(unionTypeInference.ts, 74, 17))

baz(xx);
>baz : Symbol(baz, Decl(unionTypeInference.ts, 71, 53))
>xx : Symbol(xx, Decl(unionTypeInference.ts, 74, 11))

Loading