Skip to content

Fix #30557 Narrow non-declared unions by discriminant #30593

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.

Sign up for GitHub

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

Closed
wants to merge 30 commits into from
Closed

Fix #30557 Narrow non-declared unions by discriminant #30593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e130cd6
Narrow non-declared unions by discriminant
jack-williams Mar 26, 2019
e91ee14
Only perform fallback when computed type is not an intersection or union
jack-williams Mar 26, 2019
e038290
Do not rule out intersection case
jack-williams Mar 26, 2019
74c7c92
Try reuse declaredType property cache when possible
jack-williams Mar 27, 2019
61d2eaf
Telling left and right is hard, apparently
jack-williams Mar 27, 2019
4b3a96a
Add firstDiscriminable to hit one cache
jack-williams Mar 27, 2019
8ed48e6
Merge branch 'master' into narrow-non-declared-union
RyanCavanaugh Apr 25, 2019
2293001
Set and unset root type at appropriate transition
Apr 29, 2019
a7bcf13
Do the crude thing
May 10, 2019
8ae11a3
Merge branch 'master' into narrow-non-declared-union
RyanCavanaugh Aug 15, 2019
9e8834c
Tidy up and minor fixes
jack-williams Aug 16, 2019
6d7f5e4
Selectively narrow using comparison with literal types
jack-williams Aug 29, 2019
04df882
Try and fix cycles
jack-williams Aug 29, 2019
7878514
Get rid of cycle fully
jack-williams Aug 29, 2019
7db6f39
lint
jack-williams Aug 29, 2019
d08fbb1
Merge branch 'master' into narrow-non-declared-union
jack-williams Sep 25, 2019
5278ed4
Experiment
jack-williams Sep 25, 2019
4ec69fb
Tidy
jack-williams Sep 25, 2019
c84afa3
merge branch 'master' into narrow-non-declared-union
jack-williams Oct 8, 2019
6f0ae39
More tests
jack-williams Nov 1, 2019
ed22c7b
Accept baselines and new tests
jack-williams Nov 3, 2019
2dd1f6c
More tests
jack-williams Nov 3, 2019
b2b522e
Reset and cache
jack-williams Nov 3, 2019
98ad12e
Remove union cache
jack-williams Nov 3, 2019
a3062b6
Check other cases
jack-williams Nov 3, 2019
df885ab
Merge remote-tracking branch 'upstream/master' into narrow-non-declar…
jack-williams Nov 3, 2019
fde48dd
Update baselines
jack-williams Nov 3, 2019
d798557
Try and handle join points properly
jack-williams Nov 20, 2019
1925a6c
Merge branch 'master' into narrow-non-declared-union
jack-williams Nov 20, 2019
0a037d0
Merge remote-tracking branch 'upstream/master' into narrow-non-declar…
jack-williams Nov 26, 2019
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
37 changes: 33 additions & 4 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,7 @@ namespace ts {
const symbolLinks: SymbolLinks[] = [];
const nodeLinks: NodeLinks[] = [];
const flowLoopCaches: Map<Type>[] = [];
const flowLoopContainingUnionCache: (UnionType | undefined)[] = [];
const flowLoopNodes: FlowNode[] = [];
const flowLoopKeys: string[] = [];
const flowLoopTypes: Type[][] = [];
Expand Down Expand Up @@ -19043,6 +19044,7 @@ namespace ts {
let key: string | undefined;
let keySet = false;
let flowDepth = 0;
let containingUnion: UnionType | undefined;
if (flowAnalysisDisabled) {
return errorType;
}
Expand Down Expand Up @@ -19071,7 +19073,14 @@ namespace ts {
return key = getFlowCacheKey(reference, declaredType, initialType, flowContainer);
}

function captureContainingUnion(type: FlowType, flags?: FlowFlags) {
if ((flags === undefined || (flags & FlowFlags.BranchLabel) === 0) && !isIncomplete(type) && type.flags & TypeFlags.Union) {
containingUnion = type as UnionType;
}
}

function getTypeAtFlowNode(flow: FlowNode): FlowType {
containingUnion = undefined;
if (flowDepth === 2000) {
// We have made 2000 recursive invocations. To avoid overflowing the call stack we report an error
// and disable further control flow analysis in the containing function or module body.
Expand All @@ -19089,6 +19098,7 @@ namespace ts {
for (let i = sharedFlowStart; i < sharedFlowCount; i++) {
if (sharedFlowNodes[i] === flow) {
flowDepth--;
captureContainingUnion(sharedFlowTypes[i]);
return sharedFlowTypes[i];
}
}
Expand Down Expand Up @@ -19166,6 +19176,7 @@ namespace ts {
sharedFlowTypes[sharedFlowCount] = type;
sharedFlowCount++;
}
captureContainingUnion(type, flags);
flowDepth--;
return type;
}
Expand Down Expand Up @@ -19352,6 +19363,7 @@ namespace ts {

function getTypeAtFlowBranchLabel(flow: FlowLabel): FlowType {
const antecedentTypes: Type[] = [];
const containedUnions: Type[] = [];
let subtypeReduction = false;
let seenIncomplete = false;
let bypassFlow: FlowSwitchClause | undefined;
Expand All @@ -19374,9 +19386,11 @@ namespace ts {
// are the same), there is no reason to process more antecedents since the only
// possible outcome is subtypes that will be removed in the final union type anyway.
if (type === declaredType && declaredType === initialType) {
containingUnion = undefined;
return type;
}
pushIfUnique(antecedentTypes, type);
pushIfUnique(containedUnions, containingUnion || type);
// If an antecedent type is not a subset of the declared type, we need to perform
// subtype reduction. This happens when a "foreign" type is injected into the control
// flow using the instanceof operator or a user defined type predicate.
Expand All @@ -19395,9 +19409,11 @@ namespace ts {
// the risk of circularities, we only want to perform them when they make a difference.
if (!contains(antecedentTypes, type) && !isExhaustiveSwitchStatement(bypassFlow.switchStatement)) {
if (type === declaredType && declaredType === initialType) {
containingUnion = undefined;
return type;
}
antecedentTypes.push(type);
pushIfUnique(containedUnions, containingUnion || type);
if (!isTypeSubsetOf(type, declaredType)) {
subtypeReduction = true;
}
Expand All @@ -19406,6 +19422,13 @@ namespace ts {
}
}
}
containingUnion = undefined;
captureContainingUnion(
createFlowType(
getUnionOrEvolvingArrayType(containedUnions, subtypeReduction ? UnionReduction.Subtype : UnionReduction.Literal),
seenIncomplete
)
);
return createFlowType(getUnionOrEvolvingArrayType(antecedentTypes, subtypeReduction ? UnionReduction.Subtype : UnionReduction.Literal), seenIncomplete);
}

Expand All @@ -19421,6 +19444,7 @@ namespace ts {
}
const cached = cache.get(key);
if (cached) {
containingUnion = flowLoopContainingUnionCache[id];
return cached;
}
// If this flow loop junction and reference are already being processed, return
Expand All @@ -19441,12 +19465,14 @@ namespace ts {
const antecedentTypes: Type[] = [];
let subtypeReduction = false;
let firstAntecedentType: FlowType | undefined;
let unionContainedAtTop: UnionType | undefined;
for (const antecedent of flow.antecedents!) {
let flowType;
if (!firstAntecedentType) {
// The first antecedent of a loop junction is always the non-looping control
// flow path that leads to the top.
flowType = firstAntecedentType = getTypeAtFlowNode(antecedent);
unionContainedAtTop = containingUnion;
}
else {
// All but the first antecedent are the looping control flow paths that lead
Expand All @@ -19465,6 +19491,7 @@ namespace ts {
// the resulting type and bail out.
const cached = cache.get(key);
if (cached) {
containingUnion = flowLoopContainingUnionCache[id];
return cached;
}
}
Expand All @@ -19487,8 +19514,10 @@ namespace ts {
// is incomplete.
const result = getUnionOrEvolvingArrayType(antecedentTypes, subtypeReduction ? UnionReduction.Subtype : UnionReduction.Literal);
if (isIncomplete(firstAntecedentType!)) {
containingUnion = undefined;
return createFlowType(result, /*incomplete*/ true);
}
containingUnion = flowLoopContainingUnionCache[id] = unionContainedAtTop;
cache.set(key, result);
return result;
}
Expand Down Expand Up @@ -19521,7 +19550,7 @@ namespace ts {
if (strictNullChecks && assumeTrue && optionalChainContainsReference(expr, reference)) {
type = getTypeWithFacts(type, TypeFacts.NEUndefinedOrNull);
}
if (isMatchingReferenceDiscriminant(expr, declaredType)) {
if (isMatchingReferenceDiscriminant(expr, containingUnion || declaredType)) {
return narrowTypeByDiscriminant(type, <AccessExpression>expr, t => getTypeWithFacts(t, assumeTrue ? TypeFacts.Truthy : TypeFacts.Falsy));
}
if (containsMatchingReferenceDiscriminant(reference, expr)) {
Expand Down Expand Up @@ -19580,10 +19609,10 @@ namespace ts {
type = narrowTypeByOptionalChainContainment(type, operator, left, assumeTrue);
}
}
if (isMatchingReferenceDiscriminant(left, declaredType)) {
if (isMatchingReferenceDiscriminant(left, containingUnion || declaredType)) {
return narrowTypeByDiscriminant(type, <AccessExpression>left, t => narrowTypeByEquality(t, operator, right, assumeTrue));
}
if (isMatchingReferenceDiscriminant(right, declaredType)) {
if (isMatchingReferenceDiscriminant(right, containingUnion || declaredType)) {
return narrowTypeByDiscriminant(type, <AccessExpression>right, t => narrowTypeByEquality(t, operator, left, assumeTrue));
}
if (containsMatchingReferenceDiscriminant(reference, left) || containsMatchingReferenceDiscriminant(reference, right)) {
Expand Down Expand Up @@ -19988,7 +20017,7 @@ namespace ts {
if (isMatchingReference(reference, expr)) {
return getTypeWithFacts(type, assumePresent ? TypeFacts.NEUndefinedOrNull : TypeFacts.EQUndefinedOrNull);
}
if (isMatchingReferenceDiscriminant(expr, declaredType)) {
if (isMatchingReferenceDiscriminant(expr, containingUnion || declaredType)) {
return narrowTypeByDiscriminant(type, <AccessExpression>expr, t => getTypeWithFacts(t, assumePresent ? TypeFacts.NEUndefinedOrNull : TypeFacts.EQUndefinedOrNull));
}
if (containsMatchingReferenceDiscriminant(reference, expr)) {
Expand Down
Loading