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

Assertions in control flow analysis #32695

Merged
merged 44 commits into from
Sep 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
e89acb6
Reflect effects of assertion calls in control flow analysis
ahejlsberg Jul 31, 2019
77f2a41
Support 'asserts' type predicates in control flow analysis
ahejlsberg Aug 3, 2019
1f5bb97
Remove unused code
ahejlsberg Aug 3, 2019
fe70a62
Accept new API baselines
ahejlsberg Aug 3, 2019
1c55e5d
Address code review feedback
ahejlsberg Aug 5, 2019
df02ad6
Reflect control flow effects of calls to never-returning functions
ahejlsberg Aug 8, 2019
99ab53e
Make flow nodes more monomorphic
ahejlsberg Aug 9, 2019
d5e08d4
Accept baseline API changes
ahejlsberg Aug 9, 2019
19f1d3b
Less aggressive monomorphism for flow nodes
ahejlsberg Aug 9, 2019
83212e7
Accept API baseline changes
ahejlsberg Aug 9, 2019
259ba77
Merge branch 'master' into assertionsInControlFlow
ahejlsberg Aug 9, 2019
cdeddf1
Call getResolvedSignature only when needed for generics or overloads
ahejlsberg Aug 10, 2019
9791f1d
Merge branch 'master' into assertionsInControlFlow
ahejlsberg Aug 10, 2019
0599f84
Support 'asserts this' and 'asserts this is T' type predicates
ahejlsberg Aug 17, 2019
e7cbfc4
Update API to be backwards compatible
ahejlsberg Aug 17, 2019
2c36249
Accept new API baselines
ahejlsberg Aug 17, 2019
2152874
Address CR feedback
ahejlsberg Sep 10, 2019
8791b62
Accept new baselines
ahejlsberg Sep 11, 2019
5a180ba
Merge branch 'master' into assertionsInControlFlow
ahejlsberg Sep 11, 2019
436339d
Use declared type for references in unreachable code
ahejlsberg Sep 12, 2019
a9336ba
Revert "Use declared type for references in unreachable code"
ahejlsberg Sep 12, 2019
971b0df
Use declared type for references in unreachable code (again)
ahejlsberg Sep 13, 2019
3749de6
Dedicated isReachableFlowNode function to determine reachability
ahejlsberg Sep 13, 2019
3a89c8c
Use isReachableFlowNode to check for implicit return
ahejlsberg Sep 13, 2019
cc6e493
Treat exhaustive switch statements like non-returning functions in CFA
ahejlsberg Sep 14, 2019
0060964
Further CFA handling of exhaustive switch statements
ahejlsberg Sep 15, 2019
51dcce2
Accept new baselines
ahejlsberg Sep 15, 2019
59b76ce
Fix call to Debug.fail in compiler
ahejlsberg Sep 15, 2019
945babb
Fix inference circularity error triggered by exhaustive switch analysis
ahejlsberg Sep 15, 2019
d26afd7
for-in or for-of expression is evaluated before loop back edge
ahejlsberg Sep 15, 2019
05d1e68
Fix linting issues
ahejlsberg Sep 16, 2019
e97ebb7
More efficient scheme for caching flow node reachability
ahejlsberg Sep 16, 2019
def5e37
Revert "More efficient scheme for caching flow node reachability"
ahejlsberg Sep 16, 2019
6d6c620
Report grammatic and type-based unreachable code errors in the same way
ahejlsberg Sep 16, 2019
d9c9129
Ignore references in with statements in getTypeOfDottedName
ahejlsberg Sep 16, 2019
282a7af
Accept new API baselines
ahejlsberg Sep 16, 2019
8cbf694
Cache last isReachableFlowNode result + switch statement CFA fix
ahejlsberg Sep 17, 2019
9466025
Accept new baselines
ahejlsberg Sep 17, 2019
ba30fdc
Attach flow nodes only when allowUnreachableCode !== true
ahejlsberg Sep 18, 2019
cafec55
Properly handle try-finally statements in isReachableFlowNode
ahejlsberg Sep 18, 2019
21b5418
Add tests
ahejlsberg Sep 21, 2019
97d69d4
Accept new baselines
ahejlsberg Sep 21, 2019
c3dcc37
Merge branch 'master' into assertionsInControlFlow
ahejlsberg Sep 21, 2019
bcdf33d
Fix forEachChild
ahejlsberg Sep 21, 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
54 changes: 39 additions & 15 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ namespace ts {
if (!isIIFE) {
currentFlow = { flags: FlowFlags.Start };
if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethod)) {
currentFlow.container = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
currentFlow.node = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
}
}
// We create a return control flow graph for IIFEs and constructors. For constructors
Expand All @@ -581,6 +581,7 @@ namespace ts {
if (!(currentFlow.flags & FlowFlags.Unreachable) && containerFlags & ContainerFlags.IsFunctionLike && nodeIsPresent((<FunctionLikeDeclaration>node).body)) {
node.flags |= NodeFlags.HasImplicitReturn;
if (hasExplicitReturn) node.flags |= NodeFlags.HasExplicitReturn;
(<FunctionLikeDeclaration>node).endFlowNode = currentFlow;
}
if (node.kind === SyntaxKind.SourceFile) {
node.flags |= emitFlags;
Expand Down Expand Up @@ -671,6 +672,9 @@ namespace ts {
bindJSDoc(node);
return;
}
if (node.kind >= SyntaxKind.FirstStatement && node.kind <= SyntaxKind.LastStatement && !options.allowUnreachableCode) {
node.flowNode = currentFlow;
}
switch (node.kind) {
case SyntaxKind.WhileStatement:
bindWhileStatement(<WhileStatement>node);
Expand Down Expand Up @@ -708,6 +712,9 @@ namespace ts {
case SyntaxKind.CaseClause:
bindCaseClause(<CaseClause>node);
break;
case SyntaxKind.ExpressionStatement:
bindExpressionStatement(<ExpressionStatement>node);
break;
case SyntaxKind.LabeledStatement:
bindLabeledStatement(<LabeledStatement>node);
break;
Expand Down Expand Up @@ -845,17 +852,11 @@ namespace ts {
}

function createBranchLabel(): FlowLabel {
return {
flags: FlowFlags.BranchLabel,
antecedents: undefined
};
return { flags: FlowFlags.BranchLabel, antecedents: undefined };
}

function createLoopLabel(): FlowLabel {
return {
flags: FlowFlags.LoopLabel,
antecedents: undefined
};
return { flags: FlowFlags.LoopLabel, antecedents: undefined };
}

function setFlowNodeReferenced(flow: FlowNode) {
Expand Down Expand Up @@ -885,26 +886,30 @@ namespace ts {
return antecedent;
}
setFlowNodeReferenced(antecedent);
return flowNodeCreated({ flags, expression, antecedent });
return flowNodeCreated({ flags, antecedent, node: expression });
}

function createFlowSwitchClause(antecedent: FlowNode, switchStatement: SwitchStatement, clauseStart: number, clauseEnd: number): FlowNode {
if (!isNarrowingExpression(switchStatement.expression)) {
return antecedent;
}
setFlowNodeReferenced(antecedent);
return flowNodeCreated({ flags: FlowFlags.SwitchClause, switchStatement, clauseStart, clauseEnd, antecedent });
return flowNodeCreated({ flags: FlowFlags.SwitchClause, antecedent, switchStatement, clauseStart, clauseEnd });
}

function createFlowAssignment(antecedent: FlowNode, node: Expression | VariableDeclaration | BindingElement): FlowNode {
setFlowNodeReferenced(antecedent);
return flowNodeCreated({ flags: FlowFlags.Assignment, antecedent, node });
}

function createFlowCall(antecedent: FlowNode, node: CallExpression): FlowNode {
setFlowNodeReferenced(antecedent);
return flowNodeCreated({ flags: FlowFlags.Call, antecedent, node });
}

function createFlowArrayMutation(antecedent: FlowNode, node: CallExpression | BinaryExpression): FlowNode {
setFlowNodeReferenced(antecedent);
const res: FlowArrayMutation = flowNodeCreated({ flags: FlowFlags.ArrayMutation, antecedent, node });
return res;
return flowNodeCreated({ flags: FlowFlags.ArrayMutation, antecedent, node });
}

function finishFlowLabel(flow: FlowLabel): FlowNode {
Expand Down Expand Up @@ -1030,12 +1035,12 @@ namespace ts {
function bindForInOrForOfStatement(node: ForInOrOfStatement): void {
const preLoopLabel = createLoopLabel();
const postLoopLabel = createBranchLabel();
bind(node.expression);
addAntecedent(preLoopLabel, currentFlow);
currentFlow = preLoopLabel;
if (node.kind === SyntaxKind.ForOfStatement) {
bind(node.awaitModifier);
}
bind(node.expression);
addAntecedent(postLoopLabel, currentFlow);
bind(node.initializer);
if (node.initializer.kind !== SyntaxKind.VariableDeclarationList) {
Expand Down Expand Up @@ -1222,7 +1227,8 @@ namespace ts {
addAntecedent(postSwitchLabel, currentFlow);
const hasDefault = forEach(node.caseBlock.clauses, c => c.kind === SyntaxKind.DefaultClause);
// We mark a switch statement as possibly exhaustive if it has no default clause and if all
// case clauses have unreachable end points (e.g. they all return).
// case clauses have unreachable end points (e.g. they all return). Note, we no longer need
// this property in control flow analysis, it's there only for backwards compatibility.
node.possiblyExhaustive = !hasDefault && !postSwitchLabel.antecedents;
if (!hasDefault) {
addAntecedent(postSwitchLabel, createFlowSwitchClause(preSwitchCaseFlow, node, 0, 0));
Expand Down Expand Up @@ -1281,6 +1287,24 @@ namespace ts {
activeLabels!.pop();
}

function isDottedName(node: Expression): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the difference to isEntityNameExpression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! No difference, will change to use the existing function.

return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword ||
node.kind === SyntaxKind.PropertyAccessExpression && isDottedName((<PropertyAccessExpression>node).expression) ||
node.kind === SyntaxKind.ParenthesizedExpression && isDottedName((<ParenthesizedExpression>node).expression);
}

function bindExpressionStatement(node: ExpressionStatement): void {
bind(node.expression);
// A top level call expression with a dotted function name and at least one argument
// is potentially an assertion and is therefore included in the control flow.
if (node.expression.kind === SyntaxKind.CallExpression) {
Copy link
Member

Choose a reason for hiding this comment

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

Before I forget: Even if we don't support them, we'll need tests for our behavior of:

  • Assertion constructors (arguably reasonable to support, just because construction is just calling with a replaced return value, minimally in the context of callable constructors a la js functions, it should be tested)
  • Assertion tagged templates (a tagged template is actually just a call, but given a valid template type signature, the most you could say is something like function html(text: TemplateStringsArray, holes: any[]): asserts holes[0] is Something which is... interesting.)
  • Assertion decorators (I don't even know what someone would expect here - asserting the type of the class your wrapping?)
  • Assertion JSX tags (lolwut - jsx tags are fake calls, so at best you could assert something about one of the props you're passed in?)
  • Getter/setter pairs with assertion predicates

const call = <CallExpression>node.expression;
if (isDottedName(call.expression)) {
currentFlow = createFlowCall(currentFlow, call);
}
}
}

function bindLabeledStatement(node: LabeledStatement): void {
const preStatementLabel = createLoopLabel();
const postStatementLabel = createBranchLabel();
Expand Down
Loading