improper DCE after seemingly-unrelated change

[leeoniya]

@lukastaegert

• Rollup Version: 2.9.0
• Operating System (or Browser): Windows
• Node Version (if applicable):
• Link to reproduction (IMPORTANT, read below): leeoniya/uPlot@625f600

the relevant changes are minimal (just the uPlot.js file): leeoniya/uPlot@625f600#diff-0e94ed30087b62b2ab95466722c48f84

I have not been able to get a minimal repro in the REPL :(, so hopefully the provided commit is sufficient.

Expected Behavior

instances of series.splice() are retained in the bundle, since they continue to operate on reachable variables both before and after the linked commit.

Actual Behavior

instances of series.splice() are now purged from the bundle:

leeoniya/uPlot@625f600#diff-deb943c7f48de2a8aecf97812918454aL1114-L1127

hopefully something simple (or my mistake).

thanks!

[leeoniya]

some minor movement of assignments works around this issue:

leeoniya/uPlot@abacd02#diff-0e94ed30087b62b2ab95466722c48f84

[kzc]

To repro most projects just diff the bundle generated by:

    rollup -c rollup.config.js --no-treeshake

vs

    rollup -c rollup.config.js

Then remove code from the first bundle until the test case is reduced...

$ rollup -v
rollup v2.10.0

$ cat bug.js
function f(x) {
    return (x ? [] : ["FAIL"]).map(o => o);
}
var a = f(0);
a.splice(0, 0, "PASS");
console.log(a[0]);

Expected:

$ cat bug.js | node
PASS

$ rollup bug.js --silent --no-treeshake | node
PASS

Actual:

$ rollup bug.js --silent | node
FAIL

$ rollup bug.js --silent
function f(x) {
    return (x ? [] : ["FAIL"]).map(o => o);
}
var a = f(0);
console.log(a[0]);

[kzc]

This bug also affects other mutable Array methods.

$ cat bug2.js 
function f(x) {
    return (x ? [1, 2] : [3, 4]).map(o => o);
}
var a = f(0);
a.splice(0, 0, 5);
a.reverse();
console.log(a);

Expected:

$ cat bug2.js | node
[ 4, 3, 5 ]

Actual:

$ cat bug2.js | dist/bin/rollup --silent | node
[ 3, 4 ]

$ cat bug2.js | dist/bin/rollup --silent
function f(x) {
    return (x ? [1, 2] : [3, 4]).map(o => o);
}
var a = f(0);
console.log(a);

Possible fix:

--- a/src/ast/values.ts
+++ b/src/ast/values.ts
@@ -106,7 +106,7 @@ export class UnknownArrayExpression implements ExpressionEntity {
                context: HasEffectsContext
        ) {
                if (path.length === 1) {
-                       return hasMemberEffectWhenCalled(arrayMembers, path[0], this.included, callOptions, context);
+                       return hasMemberEffectWhenCalled(arrayMembers, path[0], true, callOptions, context);
                }
                return true;
        }
@@ -323,7 +323,7 @@ export class UnknownObjectExpression implements ExpressionEntity {
                context: HasEffectsContext
        ) {
                if (path.length === 1) {
-                       return hasMemberEffectWhenCalled(objectMembers, path[0], this.included, callOptions, context);
+                       return hasMemberEffectWhenCalled(objectMembers, path[0], true, callOptions, context);
                }
                return true;
        }

With fix:

$ cat bug2.js | dist/bin/rollup --silent | node
[ 4, 3, 5 ]

[lukastaegert]

Thanks for reporting an special thanks to @kzc for that minimal repro, that was really helpful! Fix at #3559