Skip to content

Simplify deferred function, class, and accessor checking #6083

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

Merged
merged 2 commits into from
Dec 15, 2015

Conversation

ahejlsberg
Copy link
Member

We defer checking of function, class, and accessor bodies that are contained in expressions in order to avoid type checking circularities. For example:

const foo = function () {
    const s = foo();
    return "hello";
}

Here, performing a full type check of the body of the function expression whilst in the process of determining the type of foo would cause foo to be given type any because of the recursive reference. Delaying the type check of the body ensures foo has been assigned a type.

We previously performed an AST walk to find the deferred function, class, and accessor bodies. The walker was a rather brittle piece of logic with lots of cases to reason about (and possibly forget). This PR replaces the logic with a simple list of deferred nodes to be checked later. This removes a bunch of code and improves checker performance by 5% or so (with the Monaco project) due to the elimination of the extra AST walk.

The baseline changes are caused by the fact that we now never check the expression in an out-of-place return statement (as intended). We previously sometimes checked it (specifically in cases when the return statement contained a deferred function, class, or accessor body).

@@ -161,6 +161,8 @@ namespace ts {

let jsxElementClassType: Type;

let deferredNodes: Node[];
Copy link
Member

Choose a reason for hiding this comment

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

deferredCheckNodes and leave a decent comment about why these need to be deferred until the end.

Copy link
Member

Choose a reason for hiding this comment

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

Are you ever going to not have deferred nodes for most purposes? Seems like you'd have more overhead on a large project from checking over and over whether to defer than to create a one-time array allocation.

Copy link
Member Author

Choose a reason for hiding this comment

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

The deferredNodes array is used only when doing a full type check. When called from the language service (i.e. when "helicoptering in") we don't use it.

@sandersn
Copy link
Member

👍

1 similar comment
@DanielRosenwasser
Copy link
Member

👍

ahejlsberg added a commit that referenced this pull request Dec 15, 2015
Simplify deferred function, class, and accessor checking
@ahejlsberg ahejlsberg merged commit 55d4f0f into master Dec 15, 2015
@ahejlsberg ahejlsberg deleted the simplifyDeferredChecking branch December 15, 2015 17:07
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants