refactor: introduce `lookaheadInLineCharCode` #15510

JLHwung · 2023-03-23T17:51:50Z

Q	A
Tests Added + Pass?	Yes
License	MIT

In this PR we are introducing a new variant of lookaheadCharCode: lookaheadInLineCharCode. It works similar to lookaheadCharCode but will stop if a line break is seen before any source characters. It allows us to consolidate the lookaheadCharCode and hasFollowingLineBreak calls into a single lookahead operation, effectively reduces the lookahead overhead.

This PR is in preparation of the support for the Async Explicit Resource Management proposal.

JLHwung · 2023-03-23T17:53:12Z

packages/babel-parser/src/util/whitespace.ts


 export const skipWhiteSpaceInLine =
-  /(?:[^\S\n\r\u2028\u2029]|\/\/.*|\/\*.*?\*\/)*/y;
+  /(?:[^\S\n\r\u2028\u2029]|\/\/.*|\/\*.*?\*\/)*/g;


This change has no effect since we are not using .exec(), here I am just aligning to the flag of skipWhiteSpace.

babel-bot · 2023-03-23T17:54:26Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/54146/

nicolo-ribaudo

Very minor perf comment, otherwise LGTM 👍

nicolo-ribaudo · 2023-03-23T17:56:39Z

packages/babel-parser/src/parser/statement.ts

    const startsWithUsing =
-      this.isContextual(tt._using) && !this.hasFollowingLineBreak();
+      this.isContextual(tt._using) &&
+      this.hasInLineFollowingBindingIdentifier();


Can we move this check again to after startsWithUsing in line 921 below? We don't need to run it when isLetOrUsing is true because of startsWithLet.

Actually the line break check here is not required.

for (const x of []);

is valid, and so should

// Currently throwing "Missing semicolon. (1:10)" for (using x of []);

The proposal spec requires it; @rbuckton is there a reason?

Ah you are right. I overlooked the spec.

Line break is probably safe here. for (using requires a trailing semicolon to start a ForStatement, so there won't be ASI issues here from using not being a keyword.

As for this PR I will keep the current behaviour.

The proposal spec requires it; @rbuckton is there a reason?

It's definitely required in a regular for, because for reuses LexicalDeclaration, which requires it. I'd have to look into whether removing the restriction from for-of would cause problems, but either way that would be a normative change requiring consensus. I'm leaning towards leaving it as-is, however, to keep the line termination rules for using and await using consistent in all places.

keep the line termination rules for using and await using consistent in all places.

I agree. If there are any strong push back, we can always loose the restriction instead of adding one, which would be breaking.

liuxingbaoyu · 2023-03-23T18:18:21Z

packages/babel-parser/src/tokenizer/index.ts

+  /**
+   * Similar to nextToken, but it will stop at line break when it is seen before the next token
+   *
+   * @returns {number} position of the next token start or line break, whichever is seen first.
+   * @memberof Tokenizer
+   */
+  nextTokenInLineStart(): number {
+    return this.nextTokenInLineStartSince(this.state.pos);
+  }
+
+  nextTokenInLineStartSince(pos: number): number {
+    skipWhiteSpaceInLine.lastIndex = pos;
+    return skipWhiteSpaceInLine.test(this.input)
+      ? skipWhiteSpaceInLine.lastIndex
+      : pos;
+  }


nextTokenInLineStart(pos = this.state.pos): number {

Would this be better?

default parameter adds a new branching. I can inline nextTokenInLineStartSince since we are not using it anyway. (An optimizing compiler will inline it for sure).

Ah, nextTokenInLineStartSince will be used in parsing await using foo = bar(), when we do a second lookahead from using.

refactor: introduce lookaheadInLineCharCode

24b58e3

JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: parser labels Mar 23, 2023

JLHwung commented Mar 23, 2023

View reviewed changes

nicolo-ribaudo approved these changes Mar 23, 2023

View reviewed changes

liuxingbaoyu approved these changes Mar 23, 2023

View reviewed changes

address review comments

0459e64

liuxingbaoyu approved these changes Mar 23, 2023

View reviewed changes

JLHwung merged commit 656403b into babel:main Mar 24, 2023

JLHwung deleted the refactor-lookahead-check branch March 24, 2023 13:55

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2023

Uh oh!

refactor: introduce lookaheadInLineCharCode #15510

refactor: introduce lookaheadInLineCharCode #15510

Uh oh!

Conversation

JLHwung commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

babel-bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuxingbaoyu Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

refactor: introduce `lookaheadInLineCharCode` #15510

refactor: introduce `lookaheadInLineCharCode` #15510

JLHwung commented Mar 23, 2023 •

edited

Loading

babel-bot commented Mar 23, 2023 •

edited

Loading

liuxingbaoyu Mar 23, 2023 •

edited

Loading