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

Improve control flow analysis in function expressions #8849

Merged
merged 9 commits into from
May 31, 2016

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented May 26, 2016

This PR improves control flow analysis as follows:

  • Immediately invoked function expressions (IIFEs) are considered part of the surrounding control flow. Effectively, IIFEs act just like statement blocks for purposes of control flow analysis.
  • Type guards for captured const locals are in effect within nested function expressions and arrow functions. Although it is not known when (or even whether) a function expression or arrow function will be invoked, it is known that once proven true a type guard for a captured const local will remain true (because the const local can't be reassigned).

Some examples:

declare function getStringOrNumber(): string | number;

// The type guard for x remains in effect in the arrow function because x is a const local.
// Had x been declared using let or var, the type guard would not be in effect.
function f1() {
    const x = getStringOrNumber();
    if (typeof x === "string") {
        const f = () => x.length;  // Ok
    }
}

// Type guard for x remains in effect in the IIFE.
function f2() {
    let x = getStringOrNumber();
    if (typeof x === "string") {
        let n = function() {
            return x.length;  // Ok
        }();
    }
}

// Type guard for x remains in effect and assignment to y is known to
// have executed before IIFE is invoked.
function f3() {
    let x = getStringOrNumber();
    let y: number;
    if (typeof x === "string") {
        let n = (z => x.length + y + z)(y = 1);
    }
}

Fixes #8381.

@ahejlsberg
Copy link
Member Author

@mhegazy @vladima @rbuckton My latest commit optimizes the critical code paths in the binder for a bind time reduction of about 10%. Specifically, the bindChildren method had gotten really bloated and didn't put getContainerFlags to its proper full use. We'll now completely circumvent the container specific logic when the container flags are zero (which is the common case).

@ahejlsberg
Copy link
Member Author

Turns out there were more opportunities for optimization in the binder. Bind time is now reduced by 25% compared to the original code. Latest gains come from completely circumventing bindChildren for terminal nodes (such as identifiers, literals, and other token nodes). Also, the updateStrictMode function was folded into bindWorker.

const restTypes: Type[] = [];
for (let i = indexOfParameter; i < iife.arguments.length; i++) {
restTypes.push(getTypeOfExpression(iife.arguments[i]));
if (func.kind === SyntaxKind.FunctionExpression || func.kind === SyntaxKind.ArrowFunction) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we already check this in getImmediatelyInvokedFunctionExpression

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'll remove it.

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

👍

@mhegazy
Copy link
Contributor

mhegazy commented May 31, 2016

@vladima and @rbuckton can you take a look.

hasExplicitReturn = false;
currentFlow = { flags: FlowFlags.Start };
if (containerFlags & ContainerFlags.IsControlFlowContainer) {
const saveCurrentFlow = currentFlow;
Copy link
Member

Choose a reason for hiding this comment

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

Do you also need to save/restore the currentTrueTarget and currentFalseTarget flow labels here? I see that they are tracked in bindConditionalExpressionFlow and bindPrefixUnaryExpressionFlow, but what happens when one branch of the condition contains an IIFE?

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, there's no need to save those. They're only used when a conditional operator is immediately contained in a conditional statement, and they're saved and restored at that point.

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.

4 participants