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.

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

Jump to bottom

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

Merged
merged 2 commits into from
Feb 18, 2023
Merged

Switch let/const to var in the scanner & parser for top-levelish variables. #52832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
450cfe1
Switch let to var in the parser for top-levelish variables.
DanielRosenwasser Feb 17, 2023
cd8b3a6
Switch more in parser to var, switch scanner shared state to var too.
DanielRosenwasser Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 30 additions & 27 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1401,18 +1401,20 @@ export function parseJSDocTypeExpressionForTests(content: string, start?: number
// parser instances can actually be expensive enough to impact us on projects with many source
// files.
namespace Parser {
/* eslint-disable no-var */

// Share a single scanner across all calls to parse a source file. This helps speed things
// up by avoiding the cost of creating/compiling scanners over and over again.
const scanner = createScanner(ScriptTarget.Latest, /*skipTrivia*/ true);
var scanner = createScanner(ScriptTarget.Latest, /*skipTrivia*/ true);

const disallowInAndDecoratorContext = NodeFlags.DisallowInContext | NodeFlags.DecoratorContext;
var disallowInAndDecoratorContext = NodeFlags.DisallowInContext | NodeFlags.DecoratorContext;

// capture constructors in 'initializeState' to avoid null checks
let NodeConstructor: new (kind: SyntaxKind, pos: number, end: number) => Node;
let TokenConstructor: new (kind: SyntaxKind, pos: number, end: number) => Node;
let IdentifierConstructor: new (kind: SyntaxKind.Identifier, pos: number, end: number) => Identifier;
let PrivateIdentifierConstructor: new (kind: SyntaxKind.PrivateIdentifier, pos: number, end: number) => PrivateIdentifier;
let SourceFileConstructor: new (kind: SyntaxKind.SourceFile, pos: number, end: number) => SourceFile;
var NodeConstructor: new (kind: SyntaxKind, pos: number, end: number) => Node;
var TokenConstructor: new (kind: SyntaxKind, pos: number, end: number) => Node;
var IdentifierConstructor: new (kind: SyntaxKind.Identifier, pos: number, end: number) => Identifier;
var PrivateIdentifierConstructor: new (kind: SyntaxKind.PrivateIdentifier, pos: number, end: number) => PrivateIdentifier;
var SourceFileConstructor: new (kind: SyntaxKind.SourceFile, pos: number, end: number) => SourceFile;

function countNode(node: Node) {
nodeCount++;
Expand All @@ -1421,34 +1423,34 @@ namespace Parser {

// Rather than using `createBaseNodeFactory` here, we establish a `BaseNodeFactory` that closes over the
// constructors above, which are reset each time `initializeState` is called.
const baseNodeFactory: BaseNodeFactory = {
var baseNodeFactory: BaseNodeFactory = {
createBaseSourceFileNode: kind => countNode(new SourceFileConstructor(kind, /*pos*/ 0, /*end*/ 0)),
createBaseIdentifierNode: kind => countNode(new IdentifierConstructor(kind, /*pos*/ 0, /*end*/ 0)),
createBasePrivateIdentifierNode: kind => countNode(new PrivateIdentifierConstructor(kind, /*pos*/ 0, /*end*/ 0)),
createBaseTokenNode: kind => countNode(new TokenConstructor(kind, /*pos*/ 0, /*end*/ 0)),
createBaseNode: kind => countNode(new NodeConstructor(kind, /*pos*/ 0, /*end*/ 0))
};

const factory = createNodeFactory(NodeFactoryFlags.NoParenthesizerRules | NodeFactoryFlags.NoNodeConverters | NodeFactoryFlags.NoOriginalNode, baseNodeFactory);
var factory = createNodeFactory(NodeFactoryFlags.NoParenthesizerRules | NodeFactoryFlags.NoNodeConverters | NodeFactoryFlags.NoOriginalNode, baseNodeFactory);

let fileName: string;
let sourceFlags: NodeFlags;
let sourceText: string;
let languageVersion: ScriptTarget;
let scriptKind: ScriptKind;
let languageVariant: LanguageVariant;
let parseDiagnostics: DiagnosticWithDetachedLocation[];
let jsDocDiagnostics: DiagnosticWithDetachedLocation[];
let syntaxCursor: IncrementalParser.SyntaxCursor | undefined;
var fileName: string;
var sourceFlags: NodeFlags;
var sourceText: string;
var languageVersion: ScriptTarget;
var scriptKind: ScriptKind;
var languageVariant: LanguageVariant;
var parseDiagnostics: DiagnosticWithDetachedLocation[];
var jsDocDiagnostics: DiagnosticWithDetachedLocation[];
var syntaxCursor: IncrementalParser.SyntaxCursor | undefined;

let currentToken: SyntaxKind;
let nodeCount: number;
let identifiers: Map<string, string>;
let identifierCount: number;
var currentToken: SyntaxKind;
var nodeCount: number;
var identifiers: Map<string, string>;
var identifierCount: number;

let parsingContext: ParsingContext;
var parsingContext: ParsingContext;

let notParenthesizedArrow: Set<number> | undefined;
var notParenthesizedArrow: Set<number> | undefined;

// Flags that dictate what parsing context we're in. For example:
// Whether or not we are in strict parsing mode. All that changes in strict parsing mode is
Expand Down Expand Up @@ -1496,10 +1498,10 @@ namespace Parser {
// Note: it should not be necessary to save/restore these flags during speculative/lookahead
// parsing. These context flags are naturally stored and restored through normal recursive
// descent parsing and unwinding.
let contextFlags: NodeFlags;
var contextFlags: NodeFlags;

// Indicates whether we are currently parsing top-level statements.
let topLevel = true;
var topLevel = true;

// Whether or not we've had a parse error since creating the last AST node. If we have
// encountered an error, it will be stored on the next AST node we create. Parse errors
Expand Down Expand Up @@ -1528,7 +1530,8 @@ namespace Parser {
//
// Note: any errors at the end of the file that do not precede a regular node, should get
// attached to the EOF token.
let parseErrorBeforeNextFinishedNode = false;
var parseErrorBeforeNextFinishedNode = false;
/* eslint-enable no-var */

export function parseSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, syntaxCursor: IncrementalParser.SyntaxCursor | undefined, setParentNodes = false, scriptKind?: ScriptKind, setExternalModuleIndicatorOverride?: (file: SourceFile) => void): SourceFile {
scriptKind = ensureScriptKind(fileName, scriptKind);
Expand Down
25 changes: 14 additions & 11 deletions src/compiler/scanner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,31 +961,32 @@ export function createScanner(languageVersion: ScriptTarget,
start?: number,
length?: number): Scanner {

let text = textInitial!;
/* eslint-disable no-var */
var text = textInitial!;

// Current position (end position of text of current token)
let pos: number;
var pos: number;


// end of text
let end: number;
var end: number;

// Start position of whitespace before current token
let startPos: number;
var startPos: number;

// Start position of text of current token
let tokenPos: number;
var tokenPos: number;

let token: SyntaxKind;
let tokenValue!: string;
let tokenFlags: TokenFlags;
var token: SyntaxKind;
var tokenValue!: string;
var tokenFlags: TokenFlags;

let commentDirectives: CommentDirective[] | undefined;
let inJSDocType = 0;
var commentDirectives: CommentDirective[] | undefined;
var inJSDocType = 0;

setText(text, start, length);

const scanner: Scanner = {
var scanner: Scanner = {
getStartPos: () => startPos,
getTextPos: () => pos,
getToken: () => token,
Expand Down Expand Up @@ -1031,6 +1032,8 @@ export function createScanner(languageVersion: ScriptTarget,
scanRange,
};

/* eslint-disable no-var */
Copy link
Contributor

@bakkot bakkot Feb 18, 2023 •
edited

Choose a reason for hiding this comment

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

Drive-by - was this maybe supposed to be eslint-enable? It's already disabled above, and below there's no new vars.

Copy link
Member

Choose a reason for hiding this comment

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

Uh oh, sure was!


if (Debug.isDebugging) {
Object.defineProperty(scanner, "__debugShowCurrentPositionInText", {
get: () => {
Expand Down