Skip to content

Commit 4419215

Browse files
lukesandbergcopybara-github
authored andcommitted
Support @noinline on parameters and have OptimizeParameters back off when observing it.
This is necessary to support code relying on constructing objects via `.constructor` properties. Otherwise the compiler can violate application expectations by deleting or inlining parameters. PiperOrigin-RevId: 703246338
1 parent 4bf1cb0 commit 4419215

File tree

2 files changed

+36
-2
lines changed

2 files changed

+36
-2
lines changed

src/com/google/javascript/jscomp/OptimizeParameters.java

+26-2
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,9 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
670670
return !NodeUtil.doesFunctionReferenceOwnArgumentsObject(functionNode)
671671
// In `function f(a, b = a) { ... }` it's very difficult to determine if `a` is
672672
// movable.
673-
&& !mayReferenceParamBeforeBody(functionNode);
673+
&& !mayReferenceParamBeforeBody(functionNode)
674+
// `function f(/** @noinline */ a) {...}` means back off.
675+
&& !mayHaveNoInlineParameters(functionNode);
674676
}
675677
}
676678
case FUNCTION:
@@ -679,7 +681,9 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
679681
// "arguments" can refer to all parameters or their count.
680682
&& !NodeUtil.doesFunctionReferenceOwnArgumentsObject(n)
681683
// In `function f(a, b = a) { ... }` it's very difficult to determine if `a` is movable.
682-
&& !mayReferenceParamBeforeBody(n);
684+
&& !mayReferenceParamBeforeBody(n)
685+
// `function f(/** @noinline */ a) {...}` means back off.
686+
&& !mayHaveNoInlineParameters(n);
683687
case CAST:
684688
case COMMA:
685689
return allDefinitionsAreCandidateFunctions(n.getLastChild());
@@ -696,6 +700,26 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
696700
}
697701
}
698702

703+
/**
704+
* Returns true if the function has any parameters tagged with @noinline.
705+
*
706+
* <p>In this case we just back off on all parameter optimizations. We could be more clever and
707+
* only back off on the ones tagged @noinline, but it's not worth the complexity.
708+
*/
709+
private static boolean mayHaveNoInlineParameters(Node function) {
710+
Node paramList = function.getSecondChild();
711+
if (!paramList.hasChildren()) {
712+
return false; // Fast path; there can't possibly be @noinline params.
713+
}
714+
for (Node param = paramList.getFirstChild(); param != null; param = param.getNext()) {
715+
var jsDocInfo = param.getJSDocInfo();
716+
if (jsDocInfo != null && jsDocInfo.isNoInline()) {
717+
return true;
718+
}
719+
}
720+
return false;
721+
}
722+
699723
/**
700724
* Does the function use one of its parameters in code before the body?
701725
*

test/com/google/javascript/jscomp/OptimizeParametersTest.java

+10
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,7 @@ public void testSimpleRemoval1() {
409409
test(
410410
"const foo = (p1)=>{ }; foo(); foo()", //
411411
"const foo = ( )=>{var p1;}; foo(); foo()");
412+
testSame("const foo = (/** @noinline */ p1)=>{}; foo(); foo()");
412413

413414
// constant parameter
414415
test(
@@ -420,6 +421,7 @@ public void testSimpleRemoval1() {
420421
test(
421422
"const foo = (p1)=>{ }; foo(1); foo(1)",
422423
"const foo = ( )=>{var p1 = 1;}; foo( ); foo( )");
424+
testSame("const foo = (/** @noinline */ p1)=>{}; foo(1); foo(1)");
423425
}
424426

425427
@Test
@@ -439,6 +441,7 @@ public void testSimpleRemoval2() {
439441
test(
440442
"function f(p1) { } new f(1,x()); new f(1,y())",
441443
"function f( ) {var p1 = 1;} new f( x()); new f( y())");
444+
testSame("function f(/** @noinline */ p1) {} new f(); new f()");
442445
}
443446

444447
@Test
@@ -466,15 +469,22 @@ public void testSimpleRemoval4() {
466469
test(
467470
"function f(p1) { } f.prop = 1; new f(); new f()",
468471
"function f( ) {var p1;} f.prop = 1; new f(); new f()");
472+
testSame("function f(/** @noinline */ p1) {} f.prop = 1; new f(); new f()");
473+
469474
test(
470475
"function f(p1) { } f.prop = 1; new f(1); new f(1)",
471476
"function f( ) {var p1 = 1;} f.prop = 1; new f( ); new f( )");
477+
testSame("function f(/** @noinline */ p1) {} f.prop = 1; new f(1); new f(1)");
478+
472479
test(
473480
"function f(p1) { } f['prop'] = 1; new f(); new f()",
474481
"function f( ) {var p1;} f['prop'] = 1; new f(); new f()");
482+
testSame("function f(/** @noinline */ p1) {} f['prop'] = 1; new f(); new f()");
483+
475484
test(
476485
"function f(p1) { } f['prop'] = 1; new f(1); new f(1)",
477486
"function f( ) {var p1 = 1;} f['prop'] = 1; new f( ); new f( )");
487+
testSame("function f(/** @noinline */ p1) {} f['prop'] = 1; new f(1); new f(1)");
478488
}
479489

480490
@Test

0 commit comments

Comments
 (0)