Skip to content

Switch to var in binder for top level variables #52903

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

Merged
merged 2 commits into from
Feb 22, 2023
+33 −31
Merged

Switch to var in binder for top level variables #52903

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7c5b69
Switch to var in binder for top level variables
jakebailey Feb 21, 2023
8d63347
Return earlier
jakebailey Feb 22, 2023
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
64 changes: 33 additions & 31 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,49 +501,53 @@ export function bindSourceFile(file: SourceFile, options: CompilerOptions) {
}

function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
let file: SourceFile;
let options: CompilerOptions;
let languageVersion: ScriptTarget;
let parent: Node;
let container: IsContainer | EntityNameExpression;
let thisParentContainer: IsContainer | EntityNameExpression; // Container one level up
let blockScopeContainer: IsBlockScopedContainer;
let lastContainer: HasLocals;
let delayedTypeAliases: (JSDocTypedefTag | JSDocCallbackTag | JSDocEnumTag)[];
let seenThisKeyword: boolean;
/* eslint-disable no-var */
var file: SourceFile;
var options: CompilerOptions;
var languageVersion: ScriptTarget;
var parent: Node;
var container: IsContainer | EntityNameExpression;
var thisParentContainer: IsContainer | EntityNameExpression; // Container one level up
var blockScopeContainer: IsBlockScopedContainer;
var lastContainer: HasLocals;
var delayedTypeAliases: (JSDocTypedefTag | JSDocCallbackTag | JSDocEnumTag)[];
var seenThisKeyword: boolean;

// state used by control flow analysis
let currentFlow: FlowNode;
let currentBreakTarget: FlowLabel | undefined;
let currentContinueTarget: FlowLabel | undefined;
let currentReturnTarget: FlowLabel | undefined;
let currentTrueTarget: FlowLabel | undefined;
let currentFalseTarget: FlowLabel | undefined;
let currentExceptionTarget: FlowLabel | undefined;
let preSwitchCaseFlow: FlowNode | undefined;
let activeLabelList: ActiveLabel | undefined;
let hasExplicitReturn: boolean;
var currentFlow: FlowNode;
var currentBreakTarget: FlowLabel | undefined;
var currentContinueTarget: FlowLabel | undefined;
var currentReturnTarget: FlowLabel | undefined;
var currentTrueTarget: FlowLabel | undefined;
var currentFalseTarget: FlowLabel | undefined;
var currentExceptionTarget: FlowLabel | undefined;
var preSwitchCaseFlow: FlowNode | undefined;
var activeLabelList: ActiveLabel | undefined;
var hasExplicitReturn: boolean;

// state used for emit helpers
let emitFlags: NodeFlags;
var emitFlags: NodeFlags;

// If this file is an external module, then it is automatically in strict-mode according to
// ES6. If it is not an external module, then we'll determine if it is in strict mode or
// not depending on if we see "use strict" in certain places or if we hit a class/namespace
// or if compiler options contain alwaysStrict.
let inStrictMode: boolean;
var inStrictMode: boolean;

// If we are binding an assignment pattern, we will bind certain expressions differently.
let inAssignmentPattern = false;
var inAssignmentPattern = false;

let symbolCount = 0;
var symbolCount = 0;

let Symbol: new (flags: SymbolFlags, name: __String) => Symbol;
let classifiableNames: Set<__String>;
var Symbol: new (flags: SymbolFlags, name: __String) => Symbol;
var classifiableNames: Set<__String>;

const unreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
const reportedUnreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
const bindBinaryExpressionFlow = createBindBinaryExpressionFlow();
var unreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
var reportedUnreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
var bindBinaryExpressionFlow = createBindBinaryExpressionFlow();
/* eslint-enable no-var */
Copy link
Member

Choose a reason for hiding this comment

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

So one thing I was thinking with these - we should really move the return statement immediately closer to these so that it is clear that this is where shared state resides.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's a good idea; I think the checker already does this. Let me do that and rerun perf.

Copy link
Member Author

Choose a reason for hiding this comment

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

JK, it's right there, two functions down.


return bindSourceFile;

/**
* Inside the binder, we may create a diagnostic for an as-yet unbound node (with potentially no parent pointers, implying no accessible source file)
Expand Down Expand Up @@ -600,8 +604,6 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
emitFlags = NodeFlags.None;
}

return bindSourceFile;

function bindInStrictMode(file: SourceFile, opts: CompilerOptions): boolean {
if (getStrictOptionValue(opts, "alwaysStrict") && !file.isDeclarationFile) {
// bind in strict mode source files with alwaysStrict option
Expand Down