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

Refactor await/yield production parameter tracking #10956

Merged
merged 6 commits into from Feb 9, 2020
Merged

Refactor await/yield production parameter tracking #10956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
11a0981
test: add test fixtures
JLHwung Jan 2, 2020
ae0ca34
refactor: track AWAIT and YIELD in separate handler
JLHwung Jan 1, 2020
d76bfbb
fix flow errors
JLHwung Jan 3, 2020
13cb86b
add flow type annotation to production-parameter
JLHwung Jan 3, 2020
2f7be26
address review comments
JLHwung Jan 4, 2020
8102648
refactor: track [Return] parameter
JLHwung Jan 4, 2020
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
2 changes: 2 additions & 0 deletions packages/babel-parser/src/parser/base.js
View file
Open in desktop
Expand Up @@ -5,13 +5,15 @@ import type State from "../tokenizer/state";
import type { PluginsMap } from "./index";
import type ScopeHandler from "../util/scope";
import type ClassScopeHandler from "../util/class-scope";
import type ProductionParameterHandler from "../util/production-parameter";

export default class BaseParser {
// Properties set by constructor in index.js
options: Options;
inModule: boolean;
scope: ScopeHandler<*>;
classScope: ClassScopeHandler;
prodParam: ProductionParameterHandler;
plugins: PluginsMap;
filename: ?string;
sawUnambiguousESM: boolean = false;
Expand Down
62 changes: 41 additions & 21 deletions packages/babel-parser/src/parser/expression.js
View file
Open in desktop
Expand Up @@ -33,15 +33,20 @@ import * as charCodes from "charcodes";
import {
BIND_OUTSIDE,
BIND_VAR,
functionFlags,
SCOPE_ARROW,
SCOPE_CLASS,
SCOPE_DIRECT_SUPER,
SCOPE_FUNCTION,
SCOPE_SUPER,
SCOPE_PROGRAM,
SCOPE_ASYNC,
} from "../util/scopeflags";
import { ExpressionErrors } from "./util";
import {
PARAM_AWAIT,
PARAM_RETURN,
PARAM,
functionFlags,
} from "../util/production-parameter";

export default class ExpressionParser extends LValParser {
// Forward-declaration: defined in statement.js
Expand Down Expand Up @@ -106,11 +111,12 @@ export default class ExpressionParser extends LValParser {

// Convenience method to parse an Expression only
getExpression(): N.Expression {
let scopeFlags = SCOPE_PROGRAM;
let paramFlags = PARAM;
if (this.hasPlugin("topLevelAwait") && this.inModule) {
scopeFlags |= SCOPE_ASYNC;
paramFlags |= PARAM_AWAIT;
}
this.scope.enter(scopeFlags);
this.scope.enter(SCOPE_PROGRAM);
this.prodParam.enter(paramFlags);
this.nextToken();
const expr = this.parseExpression();
if (!this.match(tt.eof)) {
Expand All @@ -129,12 +135,18 @@ export default class ExpressionParser extends LValParser {
// and, *if* the syntactic construct they handle is present, wrap
// the AST node that the inner parser gave them in another node.

// Parse a full expression. The optional arguments are used to
// forbid the `in` operator (in for loops initialization expressions)
// and provide reference for storing '=' operator inside shorthand
// property assignment in contexts where both object expression
// and object pattern might appear (so it's possible to raise
// delayed syntax error at correct position).
// Parse a full expression.
// - `noIn`
// is used to forbid the `in` operator (in for loops initialization expressions)
// When `noIn` is true, the production parameter [In] is not present.
// Whenever [?In] appears in the right-hand sides of a production, we pass
// `noIn` to the subroutine calls.

// - `refExpressionErrors `
// provides reference for storing '=' operator inside shorthand
// property assignment in contexts where both object expression
// and object pattern might appear (so it's possible to raise
// delayed syntax error at correct position).

parseExpression(
noIn?: boolean,
Expand Down Expand Up @@ -167,7 +179,7 @@ export default class ExpressionParser extends LValParser {
const startPos = this.state.start;
const startLoc = this.state.startLoc;
if (this.isContextual("yield")) {
if (this.scope.inGenerator) {
if (this.prodParam.hasYield) {
let left = this.parseYield(noIn);
if (afterLeftParse) {
left = afterLeftParse.call(this, left, startPos, startLoc);
Expand Down Expand Up @@ -359,7 +371,7 @@ export default class ExpressionParser extends LValParser {
if (
this.match(tt.name) &&
this.state.value === "await" &&
this.scope.inAsync
this.prodParam.hasAwait
) {
throw this.raise(
this.state.start,
Expand Down Expand Up @@ -1180,7 +1192,7 @@ export default class ExpressionParser extends LValParser {
this.next();
meta = this.createIdentifier(meta, "function");

if (this.scope.inGenerator && this.eat(tt.dot)) {
if (this.prodParam.hasYield && this.eat(tt.dot)) {
Copy link

Choose a reason for hiding this comment

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

2x member access is expensive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but this.prodParam.hasYield is still faster than scope.inGenerator because we don't have to walkup the scope stacks to find a var scope:

babel/packages/babel-parser/src/util/scope.js

Line 209 in 09cb427

currentVarScope(): IScope {

Copy link

Choose a reason for hiding this comment

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

What about a mutual parser flag instead? state.flag |= HasYield and then
`if (state.flag & HasYield) ...``

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 think that it would make any difference? I have always assumed that we have way bigger perf problems in our parser, and that those optimizations would only be noticeable after we resolve things like "Every method access has two walk different levels of the prototype chain" and "Almost every function is megamorphic" and "Our AST objects continuously change their shape".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.prodParam.hasYield is implemented by parser flags. V8 will inline these Get accessors eventually.

The scope stack is used to track nested production params. It is equivalent to do things like

const oldHasYield = this.state.hasYield
this.state.hasYield = true;
parseFunctionBody()
this.state.hasYield = oldHasYield;

The only difference is that the hasYield above is stored implicitly in stack while this.prodParams.hasYield stored explicitly in heap.

Copy link

Choose a reason for hiding this comment

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

@nicolo-ribaudo If you see the whole picture, maybe no :) There are too many bottle necks in this parser such as the lexer, tokens itself etc.

Why not develop a new parser from bottom up? It shouldn't take more than 10 days (estimated) to replicate current Babel parser with working plugins.

Copy link
Member

Choose a reason for hiding this comment

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

If you see the even bigger picture, then it would probably be more valuable to re-implement @babel/traverse which is where Babel spends most of its time 😂

Copy link

Choose a reason for hiding this comment

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

Just looked at @babel/traverse. Should be easy :) Just get rid of the prototype and use functions instead, plus use bitmasks to handle the flow.
The findParent function is one of the root causes I guess for why it's slow :)

return this.parseMetaProperty(node, meta, "sent");
}
return this.parseFunction(node);
Expand Down Expand Up @@ -1852,13 +1864,15 @@ export default class ExpressionParser extends LValParser {
node.generator = !!isGenerator;
const allowModifiers = isConstructor; // For TypeScript parameter properties
this.scope.enter(
functionFlags(isAsync, node.generator) |
SCOPE_FUNCTION |
SCOPE_SUPER |
(inClassScope ? SCOPE_CLASS : 0) |
(allowDirectSuper ? SCOPE_DIRECT_SUPER : 0),
);
this.prodParam.enter(functionFlags(isAsync, node.generator));
this.parseFunctionParams((node: any), allowModifiers);
this.parseFunctionBodyAndFinish(node, type, true);
this.prodParam.exit();
this.scope.exit();

this.state.yieldPos = oldYieldPos;
Expand All @@ -1876,7 +1890,8 @@ export default class ExpressionParser extends LValParser {
isAsync: boolean,
trailingCommaPos: ?number,
): N.ArrowFunctionExpression {
this.scope.enter(functionFlags(isAsync, false) | SCOPE_ARROW);
this.scope.enter(SCOPE_FUNCTION | SCOPE_ARROW);
this.prodParam.enter(functionFlags(isAsync, false));
this.initFunction(node, isAsync);

const oldMaybeInArrowParameters = this.state.maybeInArrowParameters;
Expand All @@ -1889,6 +1904,7 @@ export default class ExpressionParser extends LValParser {
if (params) this.setArrowFunctionParameters(node, params, trailingCommaPos);
this.parseFunctionBody(node, true);

this.prodParam.exit();
this.scope.exit();
this.state.maybeInArrowParameters = oldMaybeInArrowParameters;
this.state.yieldPos = oldYieldPos;
Expand Down Expand Up @@ -1971,7 +1987,11 @@ export default class ExpressionParser extends LValParser {
allowExpression,
!oldStrict && useStrict,
);
// FunctionBody[Yield, Await]:
// StatementList[?Yield, ?Await, +Return] opt
this.prodParam.enter(this.prodParam.currentFlags() | PARAM_RETURN);
node.body = this.parseBlock(true, false);
this.prodParam.exit();
this.state.labels = oldLabels;
}

Expand Down Expand Up @@ -2162,7 +2182,7 @@ export default class ExpressionParser extends LValParser {
checkKeywords: boolean,
isBinding: boolean,
): void {
if (this.scope.inGenerator && word === "yield") {
if (this.prodParam.hasYield && word === "yield") {
this.raise(
startLoc,
"Can not use 'yield' as identifier inside a generator",
Expand All @@ -2171,7 +2191,7 @@ export default class ExpressionParser extends LValParser {
}

if (word === "await") {
if (this.scope.inAsync) {
if (this.prodParam.hasAwait) {
this.raise(
startLoc,
"Can not use 'await' as identifier inside an async function",
Expand Down Expand Up @@ -2209,7 +2229,7 @@ export default class ExpressionParser extends LValParser {
: isStrictReservedWord;

if (reservedTest(word, this.inModule)) {
if (!this.scope.inAsync && word === "await") {
if (!this.prodParam.hasAwait && word === "await") {
this.raise(
startLoc,
"Can not use keyword 'await' outside an async function",
Expand All @@ -2221,10 +2241,10 @@ export default class ExpressionParser extends LValParser {
}

isAwaitAllowed(): boolean {
if (this.scope.inFunction) return this.scope.inAsync;
if (this.scope.inFunction) return this.prodParam.hasAwait;
if (this.options.allowAwaitOutsideFunction) return true;
if (this.hasPlugin("topLevelAwait")) {
return this.inModule && this.scope.inAsync;
return this.inModule && this.prodParam.hasAwait;
}
return false;
}
Expand Down
14 changes: 10 additions & 4 deletions packages/babel-parser/src/parser/index.js
View file
Open in desktop
Expand Up @@ -5,9 +5,13 @@ import type { File, JSXOpeningElement } from "../types";
import type { PluginList } from "../plugin-utils";
import { getOptions } from "../options";
import StatementParser from "./statement";
import { SCOPE_ASYNC, SCOPE_PROGRAM } from "../util/scopeflags";
import { SCOPE_PROGRAM } from "../util/scopeflags";
import ScopeHandler from "../util/scope";
import ClassScopeHandler from "../util/class-scope";
import ProductionParameterHandler, {
PARAM_AWAIT,
PARAM,
} from "../util/production-parameter";

export type PluginsMap = Map<string, { [string]: any }>;

Expand All @@ -26,6 +30,7 @@ export default class Parser extends StatementParser {
this.options = options;
this.inModule = this.options.sourceType === "module";
this.scope = new ScopeHandler(this.raise.bind(this), this.inModule);
this.prodParam = new ProductionParameterHandler();
this.classScope = new ClassScopeHandler(this.raise.bind(this));
this.plugins = pluginsMap(this.options.plugins);
this.filename = options.sourceFilename;
Expand All @@ -37,11 +42,12 @@ export default class Parser extends StatementParser {
}

parse(): File {
let scopeFlags = SCOPE_PROGRAM;
let paramFlags = PARAM;
if (this.hasPlugin("topLevelAwait") && this.inModule) {
scopeFlags |= SCOPE_ASYNC;
paramFlags |= PARAM_AWAIT;
}
this.scope.enter(scopeFlags);
this.scope.enter(SCOPE_PROGRAM);
this.prodParam.enter(paramFlags);
const file = this.startNode();
const program = this.startNode();
this.nextToken();
Expand Down
15 changes: 12 additions & 3 deletions packages/babel-parser/src/parser/statement.js
View file
Open in desktop
Expand Up @@ -15,8 +15,8 @@ import {
BIND_LEXICAL,
BIND_VAR,
BIND_FUNCTION,
functionFlags,
SCOPE_CLASS,
SCOPE_FUNCTION,
SCOPE_OTHER,
SCOPE_SIMPLE_CATCH,
SCOPE_SUPER,
Expand All @@ -28,6 +28,7 @@ import {
type BindingTypes,
} from "../util/scopeflags";
import { ExpressionErrors } from "./util";
import { PARAM, functionFlags } from "../util/production-parameter";

const loopLabel = { kind: "loop" },
switchLabel = { kind: "switch" };
Expand Down Expand Up @@ -574,7 +575,7 @@ export default class StatementParser extends ExpressionParser {
}

parseReturnStatement(node: N.ReturnStatement): N.ReturnStatement {
if (!this.scope.inFunction && !this.options.allowReturnOutsideFunction) {
if (!this.prodParam.hasReturn && !this.options.allowReturnOutsideFunction) {
this.raise(this.state.start, "'return' outside of function");
}

Expand Down Expand Up @@ -1059,7 +1060,8 @@ export default class StatementParser extends ExpressionParser {
this.state.maybeInArrowParameters = false;
this.state.yieldPos = -1;
this.state.awaitPos = -1;
this.scope.enter(functionFlags(node.async, node.generator));
this.scope.enter(SCOPE_FUNCTION);
this.prodParam.enter(functionFlags(isAsync, node.generator));

if (!isStatement) {
node.id = this.parseFunctionId();
Expand All @@ -1078,6 +1080,7 @@ export default class StatementParser extends ExpressionParser {
);
});

this.prodParam.exit();
this.scope.exit();

if (isStatement && !isHangingStatement) {
Expand Down Expand Up @@ -1599,9 +1602,12 @@ export default class StatementParser extends ExpressionParser {
node: N.ClassPrivateProperty,
): N.ClassPrivateProperty {
this.scope.enter(SCOPE_CLASS | SCOPE_SUPER);
// [In] production parameter is tracked in parseMaybeAssign
this.prodParam.enter(PARAM);

node.value = this.eat(tt.eq) ? this.parseMaybeAssign() : null;
this.semicolon();
this.prodParam.exit();

this.scope.exit();

Expand All @@ -1614,6 +1620,8 @@ export default class StatementParser extends ExpressionParser {
}

this.scope.enter(SCOPE_CLASS | SCOPE_SUPER);
// [In] production parameter is tracked in parseMaybeAssign
this.prodParam.enter(PARAM);

if (this.match(tt.eq)) {
this.expectPlugin("classProperties");
Expand All @@ -1624,6 +1632,7 @@ export default class StatementParser extends ExpressionParser {
}
this.semicolon();

this.prodParam.exit();
this.scope.exit();

return this.finishNode(node, "ClassProperty");
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-parser/src/plugins/flow.js
View file
Open in desktop
Expand Up @@ -12,13 +12,13 @@ import { types as tc } from "../tokenizer/context";
import * as charCodes from "charcodes";
import { isIteratorStart } from "../util/identifier";
import {
functionFlags,
type BindingTypes,
BIND_NONE,
BIND_LEXICAL,
BIND_VAR,
BIND_FUNCTION,
SCOPE_ARROW,
SCOPE_FUNCTION,
SCOPE_OTHER,
} from "../util/scopeflags";
import type { ExpressionErrors } from "../parser/util";
Expand Down Expand Up @@ -1891,7 +1891,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
node.extra?.trailingComma,
);
// Enter scope, as checkParams defines bindings
this.scope.enter(functionFlags(false, false) | SCOPE_ARROW);
this.scope.enter(SCOPE_FUNCTION | SCOPE_ARROW);
// Use super's method to force the parameters to be checked
super.checkParams(node, false, true);
this.scope.exit();
Expand Down
7 changes: 7 additions & 0 deletions packages/babel-parser/src/plugins/typescript/index.js
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@ import {
import TypeScriptScopeHandler from "./scope";
import * as charCodes from "charcodes";
import type { ExpressionErrors } from "../../parser/util";
import { PARAM } from "../../util/production-parameter";

type TsModifier =
| "readonly"
Expand Down Expand Up @@ -1265,7 +1266,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>
node.body = inner;
} else {
this.scope.enter(SCOPE_TS_MODULE);
this.prodParam.enter(PARAM);
node.body = this.tsParseModuleBlock();
this.prodParam.exit();
this.scope.exit();
}
return this.finishNode(node, "TSModuleDeclaration");
Expand All @@ -1284,7 +1287,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>
}
if (this.match(tt.braceL)) {
this.scope.enter(SCOPE_TS_MODULE);
this.prodParam.enter(PARAM);
node.body = this.tsParseModuleBlock();
this.prodParam.exit();
this.scope.exit();
} else {
this.semicolon();
Expand Down Expand Up @@ -1439,11 +1444,13 @@ export default (superClass: Class<Parser>): Class<Parser> =>
// Would like to use tsParseAmbientExternalModuleDeclaration here, but already ran past "global".
if (this.match(tt.braceL)) {
this.scope.enter(SCOPE_TS_MODULE);
this.prodParam.enter(PARAM);
const mod: N.TsModuleDeclaration = node;
mod.global = true;
mod.id = expr;
mod.body = this.tsParseModuleBlock();
this.scope.exit();
this.prodParam.exit();
return this.finishNode(mod, "TSModuleDeclaration");
}
break;
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-parser/src/tokenizer/context.js
View file
Open in desktop
Expand Up @@ -60,7 +60,7 @@ tt.name.updateContext = function(prevType) {
if (prevType !== tt.dot) {
if (
(this.state.value === "of" && !this.state.exprAllowed) ||
(this.state.value === "yield" && this.scope.inGenerator)
(this.state.value === "yield" && this.prodParam.hasYield)
) {
allowed = true;
}
Expand Down