Skip to content

Commit

Permalink
Don't try to inline function expression calls with spread arguments
Browse files Browse the repository at this point in the history
Moves the check for spread arguments into FunctionInjector.canInlineReferenceToFunction, since apparently InlineFunctions.isCandidateUsage is not called for IIFEs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=187210607
  • Loading branch information
lauraharker authored and Tyler Breisacher committed Feb 27, 2018
1 parent 22b2bbb commit 947a994
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 15 deletions.
16 changes: 16 additions & 0 deletions src/com/google/javascript/jscomp/FunctionInjector.java
Expand Up @@ -199,6 +199,10 @@ CanInlineResult canInlineReferenceToFunction(
return CanInlineResult.NO;
}

if (hasSpreadCallArgument(callNode)) {
return CanInlineResult.NO;
}

// Limit where functions that contain functions can be inline. Introducing
// an inner function into another function can capture a variable and cause
// a memory leak. This isn't a problem in the global scope as those values
Expand Down Expand Up @@ -250,6 +254,18 @@ private boolean isSupportedCallType(Node callNode) {
return true;
}

private static boolean hasSpreadCallArgument(Node callNode) {
Predicate<Node> hasSpreadCallArgumentPredicate =
new Predicate<Node>() {
@Override
public boolean apply(Node input) {
return input.isSpread();
}
};

return NodeUtil.has(callNode, hasSpreadCallArgumentPredicate, Predicates.<Node>alwaysTrue());
}

/**
* Inline a function into the call site.
*/
Expand Down
15 changes: 0 additions & 15 deletions src/com/google/javascript/jscomp/InlineFunctions.java
Expand Up @@ -454,9 +454,6 @@ static boolean isCandidateUsage(Node name) {
}

if (parent.isCall() && parent.getFirstChild() == name) {
if (hasSpreadCallArgument(parent)) {
return false;
}
// This is a normal reference to the function.
return true;
}
Expand Down Expand Up @@ -722,18 +719,6 @@ private void resolveInlineConflicts() {
}
}

private static boolean hasSpreadCallArgument(Node callNode) {
Predicate<Node> hasSpreadCallArgumentPredicate =
new Predicate<Node>() {
@Override
public boolean apply(Node input) {
return input.isSpread();
}
};

return NodeUtil.has(callNode, hasSpreadCallArgumentPredicate, Predicates.<Node>alwaysTrue());
}

/**
* @return Whether the function has any parameters that would stop the compiler from inlining.
* Currently this includes object patterns, array patterns, and default values.
Expand Down
11 changes: 11 additions & 0 deletions test/com/google/javascript/jscomp/InlineFunctionsTest.java
Expand Up @@ -3307,6 +3307,17 @@ public void testSpreadCall() {
" return args.length;",
"}",
"foo(...[0,1]);"));

testSame("var args = [0, 1]; (function foo(x, y) { return x + y; })(...args);");

testSame(
lines(
"var args = [0, 1];",
"(function foo(x, y, z) {",
" return x + y + z;",
"})(2, ...args);"));

testSame("(function (x, y) { return x + y; })(...[0, 1]);");
}

public void testGeneratorFunction() {
Expand Down

0 comments on commit 947a994

Please sign in to comment.