-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Control flow analysis for array construction #11432
Changes from 17 commits
9ca2569
b5c1055
9c4190b
144f4ff
a9c2b23
c0c2271
b8d5226
12906a7
2f5af2e
71b9b33
d202b1c
89826a5
e6b588a
612ed1e
e9858de
c9e2f95
38278ee
1dedca7
620b3f9
a27a68f
bf301e9
bfa4197
79ed3a7
c876d92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1907,8 +1907,9 @@ namespace ts { | |
TrueCondition = 1 << 5, // Condition known to be true | ||
FalseCondition = 1 << 6, // Condition known to be false | ||
SwitchClause = 1 << 7, // Switch statement clause | ||
Referenced = 1 << 8, // Referenced as antecedent once | ||
Shared = 1 << 9, // Referenced as antecedent more than once | ||
ArrayMutation = 1 << 8, // Potential array mutation | ||
Referenced = 1 << 9, // Referenced as antecedent once | ||
Shared = 1 << 10, // Referenced as antecedent more than once | ||
Label = BranchLabel | LoopLabel, | ||
Condition = TrueCondition | FalseCondition | ||
} | ||
|
@@ -1951,6 +1952,13 @@ namespace ts { | |
antecedent: FlowNode; | ||
} | ||
|
||
// FlowArrayMutation represents a node potentially mutates an array, i.e. an | ||
// operation of the form 'x.push(value)' or 'x[n] = value'. | ||
export interface FlowArrayMutation extends FlowNode { | ||
node: CallExpression | BinaryExpression; | ||
antecedent: FlowNode; | ||
} | ||
|
||
export type FlowType = Type | IncompleteType; | ||
|
||
// Incomplete types occur during control flow analysis of loops. An IncompleteType | ||
|
@@ -2748,6 +2756,8 @@ namespace ts { | |
export interface AnonymousType extends ObjectType { | ||
target?: AnonymousType; // Instantiation target | ||
mapper?: TypeMapper; // Instantiation mapper | ||
elementType?: Type; // Element expressions of evolving array type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why isn't AutoArrayType a new subtype of AnonymousType? I think it would make it easier to track the property that auto array types don't escape the dynamic scope of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't follow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's separate from I'm talking about the comment at the top of the new code that says
I was suggesting something like: export interface AutoArrayType extends AnonymousType {
elementType?: Type;
finalArrayType?: Type;
} And then having the new code that returns AnonymousType today return AutoArrayType instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see. The issue with doing it that way is that we don't have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, that makes sense. |
||
finalArrayType?: Type; // Final array type of evolving array type | ||
} | ||
|
||
/* @internal */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
tests/cases/compiler/controlFlowArrayErrors.ts(6,9): error TS7005: Variable 'y' implicitly has an 'any[]' type. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(14,9): error TS7005: Variable 'y' implicitly has an 'any[]' type. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(20,9): error TS7034: Variable 'x' implicitly has type 'any[]' in some locations where its type cannot be determined. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(23,9): error TS7005: Variable 'x' implicitly has an 'any[]' type. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(30,12): error TS2345: Argument of type 'true' is not assignable to parameter of type 'string | number'. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(35,12): error TS2345: Argument of type 'true' is not assignable to parameter of type 'string | number'. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(49,5): error TS2349: Cannot invoke an expression whose type lacks a call signature. Type '((...items: (string | number)[]) => number) | ((...items: boolean[]) => number)' has no compatible call signatures. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(57,12): error TS2345: Argument of type '"hello"' is not assignable to parameter of type 'number'. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(61,11): error TS7034: Variable 'x' implicitly has type 'any[]' in some locations where its type cannot be determined. | ||
tests/cases/compiler/controlFlowArrayErrors.ts(64,9): error TS7005: Variable 'x' implicitly has an 'any[]' type. | ||
|
||
|
||
==== tests/cases/compiler/controlFlowArrayErrors.ts (10 errors) ==== | ||
|
||
declare function cond(): boolean; | ||
|
||
function f1() { | ||
let x = []; | ||
let y = x; // Implicit any[] error | ||
~ | ||
!!! error TS7005: Variable 'y' implicitly has an 'any[]' type. | ||
x.push(5); | ||
let z = x; | ||
} | ||
|
||
function f2() { | ||
let x; | ||
x = []; | ||
let y = x; // Implicit any[] error | ||
~ | ||
!!! error TS7005: Variable 'y' implicitly has an 'any[]' type. | ||
x.push(5); | ||
let z = x; | ||
} | ||
|
||
function f3() { | ||
let x = []; // Implicit any[] error in some locations | ||
~ | ||
!!! error TS7034: Variable 'x' implicitly has type 'any[]' in some locations where its type cannot be determined. | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
~ | ||
!!! error TS7005: Variable 'x' implicitly has an 'any[]' type. | ||
} | ||
} | ||
|
||
function f4() { | ||
let x; | ||
x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
~~~~ | ||
!!! error TS2345: Argument of type 'true' is not assignable to parameter of type 'string | number'. | ||
} | ||
|
||
function f5() { | ||
let x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
~~~~ | ||
!!! error TS2345: Argument of type 'true' is not assignable to parameter of type 'string | number'. | ||
} | ||
|
||
function f6() { | ||
let x; | ||
if (cond()) { | ||
x = []; | ||
x.push(5); | ||
x.push("hello"); | ||
} | ||
else { | ||
x = [true]; // Non-evolving array | ||
} | ||
x; // boolean[] | (string | number)[] | ||
x.push(99); // Error | ||
~~~~~~~~~~ | ||
!!! error TS2349: Cannot invoke an expression whose type lacks a call signature. Type '((...items: (string | number)[]) => number) | ((...items: boolean[]) => number)' has no compatible call signatures. | ||
} | ||
|
||
function f7() { | ||
let x = []; // x has evolving array value | ||
x.push(5); | ||
let y = x; // y has non-evolving array value | ||
x.push("hello"); // Ok | ||
y.push("hello"); // Error | ||
~~~~~~~ | ||
!!! error TS2345: Argument of type '"hello"' is not assignable to parameter of type 'number'. | ||
} | ||
|
||
function f8() { | ||
const x = []; // Implicit any[] error in some locations | ||
~ | ||
!!! error TS7034: Variable 'x' implicitly has type 'any[]' in some locations where its type cannot be determined. | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
~ | ||
!!! error TS7005: Variable 'x' implicitly has an 'any[]' type. | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
//// [controlFlowArrayErrors.ts] | ||
|
||
declare function cond(): boolean; | ||
|
||
function f1() { | ||
let x = []; | ||
let y = x; // Implicit any[] error | ||
x.push(5); | ||
let z = x; | ||
} | ||
|
||
function f2() { | ||
let x; | ||
x = []; | ||
let y = x; // Implicit any[] error | ||
x.push(5); | ||
let z = x; | ||
} | ||
|
||
function f3() { | ||
let x = []; // Implicit any[] error in some locations | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
} | ||
} | ||
|
||
function f4() { | ||
let x; | ||
x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
} | ||
|
||
function f5() { | ||
let x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
} | ||
|
||
function f6() { | ||
let x; | ||
if (cond()) { | ||
x = []; | ||
x.push(5); | ||
x.push("hello"); | ||
} | ||
else { | ||
x = [true]; // Non-evolving array | ||
} | ||
x; // boolean[] | (string | number)[] | ||
x.push(99); // Error | ||
} | ||
|
||
function f7() { | ||
let x = []; // x has evolving array value | ||
x.push(5); | ||
let y = x; // y has non-evolving array value | ||
x.push("hello"); // Ok | ||
y.push("hello"); // Error | ||
} | ||
|
||
function f8() { | ||
const x = []; // Implicit any[] error in some locations | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
} | ||
} | ||
|
||
//// [controlFlowArrayErrors.js] | ||
function f1() { | ||
var x = []; | ||
var y = x; // Implicit any[] error | ||
x.push(5); | ||
var z = x; | ||
} | ||
function f2() { | ||
var x; | ||
x = []; | ||
var y = x; // Implicit any[] error | ||
x.push(5); | ||
var z = x; | ||
} | ||
function f3() { | ||
var x = []; // Implicit any[] error in some locations | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
} | ||
} | ||
function f4() { | ||
var x; | ||
x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
} | ||
function f5() { | ||
var x = [5, "hello"]; // Non-evolving array | ||
x.push(true); // Error | ||
} | ||
function f6() { | ||
var x; | ||
if (cond()) { | ||
x = []; | ||
x.push(5); | ||
x.push("hello"); | ||
} | ||
else { | ||
x = [true]; // Non-evolving array | ||
} | ||
x; // boolean[] | (string | number)[] | ||
x.push(99); // Error | ||
} | ||
function f7() { | ||
var x = []; // x has evolving array value | ||
x.push(5); | ||
var y = x; // y has non-evolving array value | ||
x.push("hello"); // Ok | ||
y.push("hello"); // Error | ||
} | ||
function f8() { | ||
var x = []; // Implicit any[] error in some locations | ||
x.push(5); | ||
function g() { | ||
x; // Implicit any[] error | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about
unshift
? I used unshift briefly when writing spread types. :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, we should support
unshift
, uncommon as it may be.