Skip to content

Commit

Permalink
fix edge case in scope analysis
Browse files Browse the repository at this point in the history 
fix double declaration problem in variable declarations
remove TrackingSet
rename StackedSetMap to StackedMap and remove add method
add more scope analysis test
  • Loading branch information
@sokra
sokra committed Sep 5, 2019
1 parent acc7c3e commit ec51894
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 173 deletions.
97 changes: 46 additions & 51 deletions lib/JavascriptParser.js
View file
Expand Up @@ -9,19 +9,21 @@ const { Parser } = require("acorn");
const { SyncBailHook, HookMap } = require("tapable");
const vm = require("vm");
const BasicEvaluatedExpression = require("./BasicEvaluatedExpression");
const StackedSetMap = require("./util/StackedSetMap");
const StackedMap = require("./util/StackedMap");

// Syntax: https://developer.mozilla.org/en/SpiderMonkey/Parser_API

const parser = Parser.extend(require("./parsing/importAwaitAcornPlugin"));

/**
* @typedef {Object} VariableInfo
* @property {string | true} freeName
* @property {TagInfo} tagInfo
*/
class VariableInfo {
constructor(declaredScope, freeName, tagInfo) {
this.declaredScope = declaredScope;
this.freeName = freeName;
this.tagInfo = tagInfo;
}
}

/** @typedef {string | true | VariableInfo} ExportedVariableInfo */
/** @typedef {string | ScopeInfo | VariableInfo} ExportedVariableInfo */

/**
* @typedef {Object} TagInfo
Expand All @@ -32,7 +34,7 @@ const parser = Parser.extend(require("./parsing/importAwaitAcornPlugin"));

/**
* @typedef {Object} ScopeInfo
* @property {StackedSetMap<string, VariableInfo | true>} definitions
* @property {StackedMap<string, VariableInfo | ScopeInfo>} definitions
* @property {boolean | "arrow"} topLevelScope
* @property {boolean} inShorthand
* @property {boolean} isStrict
Expand Down Expand Up @@ -2132,16 +2134,16 @@ class JavascriptParser {
* @returns {any} resut of hook
*/
callHooksForInfoWithFallback(hookMap, info, fallback, defined, ...args) {
if (info === true) {
if (defined !== undefined) {
return defined();
}
return;
}
let name;
if (typeof info === "string") {
name = info;
} else {
if (!(info instanceof VariableInfo)) {
if (defined !== undefined) {
return defined();
}
return;
}
let tagInfo = info.tagInfo;
while (tagInfo !== undefined) {
const hook = hookMap.get(tagInfo.tag);
Expand Down Expand Up @@ -2474,7 +2476,7 @@ class JavascriptParser {
inTry: false,
inShorthand: false,
isStrict: false,
definitions: new StackedSetMap()
definitions: new StackedMap()
};
const state = (this.state = initialState || {});
this.comments = comments;
Expand Down Expand Up @@ -2529,7 +2531,7 @@ class JavascriptParser {

getTagData(name, tag) {
const info = this.scope.definitions.get(name);
if (typeof info === "object") {
if (info instanceof VariableInfo) {
let tagInfo = info.tagInfo;
while (tagInfo !== undefined) {
if (tagInfo.tag === tag) return tagInfo.data;
Expand All @@ -2543,38 +2545,33 @@ class JavascriptParser {
/** @type {VariableInfo} */
let newInfo;
if (oldInfo === undefined) {
newInfo = {
freeName: name,
tagInfo: {
tag,
data,
next: undefined
}
};
} else if (oldInfo === true) {
newInfo = {
freeName: true,
tagInfo: {
tag,
data,
next: undefined
}
};
newInfo = new VariableInfo(this.scope, name, {
tag,
data,
next: undefined
});
} else if (oldInfo instanceof VariableInfo) {
newInfo = new VariableInfo(oldInfo.declaredScope, oldInfo.freeName, {
tag,
data,
next: oldInfo.tagInfo
});
} else {
newInfo = {
freeName: oldInfo.freeName,
tagInfo: {
tag,
data,
next: oldInfo.tagInfo
}
};
newInfo = new VariableInfo(oldInfo, true, {
tag,
data,
next: undefined
});
}
this.scope.definitions.set(name, newInfo);
}

defineVariable(name) {
this.scope.definitions.set(name, true);
const oldInfo = this.scope.definitions.get(name);
// Don't redefine variable in same scope to keep existing tags
if (oldInfo instanceof VariableInfo && oldInfo.declaredScope === this.scope)
return;
this.scope.definitions.set(name, this.scope);
}

undefineVariable(name) {
Expand All @@ -2589,8 +2586,6 @@ class JavascriptParser {
const value = this.scope.definitions.get(name);
if (value === undefined) {
return name;
} else if (value === true) {
return true;
} else {
return value;
}
Expand All @@ -2606,10 +2601,10 @@ class JavascriptParser {
if (variableInfo === name) {
this.scope.definitions.delete(name);
} else {
this.scope.definitions.set(name, {
freeName: variableInfo,
tagInfo: undefined
});
this.scope.definitions.set(
name,
new VariableInfo(this.scope, variableInfo, undefined)
);
}
} else {
this.scope.definitions.set(name, variableInfo);
Expand Down Expand Up @@ -2664,14 +2659,14 @@ class JavascriptParser {
return undefined;
}
const rootInfo = this.getVariableInfo(rootName);
/** @type {string | true} */
/** @type {string | ScopeInfo} */
let resolvedRoot;
if (typeof rootInfo === "object") {
if (rootInfo instanceof VariableInfo) {
resolvedRoot = rootInfo.freeName;
} else {
resolvedRoot = rootInfo;
}
if (resolvedRoot === true) {
if (typeof resolvedRoot !== "string") {
return undefined;
}
let name = resolvedRoot;
Expand Down
62 changes: 42 additions & 20 deletions lib/optimize/InnerGraphPlugin.js
View file
Expand Up @@ -15,6 +15,20 @@ const topLevelSymbolTag = Symbol("top level symbol");

/** @typedef {Map<TopLevelSymbol | Dependency, Set<string | TopLevelSymbol> | true>} InnerGraph */

const isPure = expr => {
switch (expr.type) {
case "Identifier":
return true;
case "Literal":
return true;
case "ConditionalExpression":
return (
isPure(expr.test) && isPure(expr.consequent) && isPure(expr.alternate)
);
}
return false;
};

class TopLevelSymbol {
/**
* @param {string} name name of the function
Expand Down Expand Up @@ -65,7 +79,7 @@ class InnerGraphPlugin {
parser.state.harmonyAllExportDependentDependencies = new Set();
});

parser.hooks.finish.tap("HarmonyImportDependencyParserPlugin", () => {
parser.hooks.finish.tap("InnerGraphPlugin", () => {
const innerGraph =
/** @type {InnerGraph} */ (parser.state.harmonyInnerGraph);
if (!innerGraph) return;
Expand Down Expand Up @@ -158,14 +172,26 @@ class InnerGraphPlugin {
) {
const innerGraph =
/** @type {InnerGraph} */ (parser.state.harmonyInnerGraph);
parser.defineVariable("*default*");
const fn = new TopLevelSymbol("*default*", innerGraph);
parser.tagVariable("*default*", topLevelSymbolTag, fn);
const name = "*default*";
parser.defineVariable(name);
const fn = new TopLevelSymbol(name, innerGraph);
parser.tagVariable(name, topLevelSymbolTag, fn);
statementWithTopLevelSymbol.set(statement, fn);
}
}
}
});
const tagVar = name => {
const innerGraph =
/** @type {InnerGraph} */ (parser.state.harmonyInnerGraph);
parser.defineVariable(name);
const existingTag = parser.getTagData(name, topLevelSymbolTag);
const fn = existingTag || new TopLevelSymbol(name, innerGraph);
if (!existingTag) {
parser.tagVariable(name, topLevelSymbolTag, fn);
}
return fn;
};
/** @type {WeakMap<{}, TopLevelSymbol>} */
const declWithTopLevelSymbol = new WeakMap();
parser.hooks.preDeclarator.tap(
Expand All @@ -181,30 +207,26 @@ class InnerGraphPlugin {
decl.init.type === "ArrowFunctionExpression" ||
decl.init.type === "ClassExpression"
) {
const innerGraph =
/** @type {InnerGraph} */ (parser.state.harmonyInnerGraph);
parser.defineVariable(decl.id.name);
const fn = new TopLevelSymbol(decl.id.name, innerGraph);
parser.tagVariable(decl.id.name, topLevelSymbolTag, fn);
const name = decl.id.name;
const fn = tagVar(name);
declWithTopLevelSymbol.set(decl, fn);
return true;
}
if (
decl.init.range[0] - decl.id.range[1] > 9 &&
parser
.getComments([decl.id.range[1], decl.init.range[0]])
.some(
comment =>
comment.type === "Block" &&
/^\s*(#|@)__PURE__\s*$/.test(comment.value)
)
(decl.init.range[0] - decl.id.range[1] > 9 &&
parser
.getComments([decl.id.range[1], decl.init.range[0]])
.some(
comment =>
comment.type === "Block" &&
/^\s*(#|@)__PURE__\s*$/.test(comment.value)
)) ||
isPure(decl.init)
) {
const innerGraph =
/** @type {InnerGraph} */ (parser.state.harmonyInnerGraph);
const name = decl.id.name;
parser.defineVariable(name);
const fn = new TopLevelSymbol(name, innerGraph);
parser.tagVariable(name, topLevelSymbolTag, fn);
const fn = tagVar(name);
declWithTopLevelSymbol.set(decl, fn);
const dep = new PureExpressionDependency(decl.init.range);
dep.loc = decl.loc;
Expand Down
18 changes: 9 additions & 9 deletions lib/optimize/ModuleConcatenationPlugin.js
View file
Expand Up @@ -12,7 +12,7 @@ const HarmonyCompatibilityDependency = require("../dependencies/HarmonyCompatibi
const HarmonyImportDependency = require("../dependencies/HarmonyImportDependency");
const ModuleHotAcceptDependency = require("../dependencies/ModuleHotAcceptDependency");
const ModuleHotDeclineDependency = require("../dependencies/ModuleHotDeclineDependency");
const StackedSetMap = require("../util/StackedSetMap");
const StackedMap = require("../util/StackedMap");
const ConcatenatedModule = require("./ConcatenatedModule");

/** @typedef {import("../Compilation")} Compilation */
Expand Down Expand Up @@ -487,21 +487,21 @@ class ConcatConfiguration {
constructor(rootModule, cloneFrom) {
this.rootModule = rootModule;
if (cloneFrom) {
/** @type {StackedSetMap} */
/** @type {StackedMap<Module, true>} */
this.modules = cloneFrom.modules.createChild();
/** @type {StackedSetMap} */
/** @type {StackedMap<Module, Module>} */
this.warnings = cloneFrom.warnings.createChild();
} else {
/** @type {StackedSetMap} */
this.modules = new StackedSetMap();
this.modules.add(rootModule);
/** @type {StackedSetMap} */
this.warnings = new StackedSetMap();
/** @type {StackedMap<Module, true>} */
this.modules = new StackedMap();
this.modules.set(rootModule, true);
/** @type {StackedMap<Module, Module>} */
this.warnings = new StackedMap();
}
}

add(module) {
this.modules.add(module);
this.modules.set(module, true);
}

has(module) {
Expand Down

0 comments on commit ec51894

Please sign in to comment.