closure-compiler
closure-compiler copied to clipboard
`@pureOrBreakMyCode` can't handle when left-hand side is array pattern
command java -jar closure-compiler.jar --compilation_level ADVANCED_OPTIMIZATIONS --formatting PRETTY_PRINT --js input.js --js_output_file compiled.js
the example code is trimmed from tc39 stage 3 decorator compiled by swc.
in the example,@pureOrBreakMyCode is added manually to ({ e: [_init_p3] } = /** @pureOrBreakMyCode */_apply_decs_2203_r(...)
however, because left-hand side is destructuring assignment, so closure-compiler need to traverse the lhs and valueNode respectively, it can't eliminate the pure valueNode.
reproducible code
/** @noinline */
const applyDecs2203RFactory = () => { return () => {} }
/** @noinline */
const _apply_decs_2203_r = applyDecs2203RFactory()
var _init_p3;
class A {
constructor(){
this.p3 = /** @pureOrBreakMyCode */_init_p3(this, "3");
}
}
({ e: [_init_p3] } = /** @pureOrBreakMyCode */_apply_decs_2203_r(1, 2, 3));
I'm seeing that the LHS is eliminated even without the @pureOrBreakMyCode annotation. Why do you think this is specific to the annotation?
Hi @rishipal thanks for looking into this
When @pureOrBreakMyCode is used, I expect the rhs gets removed with lhs together, which is the case when lhs is a name
in this example, when lhs is name, the whole code can be eliminated
but when lhs is destructuring assignment, it's not the case
It would be nice to have RemoveUnusedCode recognize this annotation and remove the call. Perhaps the /** @pureOrBreakMyCode */ isn't getting attached correctly and needs parens?
In any case, we must better document this annotation for ourselves. @rahul-kamat will take a look at your PR.
Thanks @rishipal and @rahul-kamat
unfortunately my employer Canva doesn't have a CLA signed with Google right now, so I guess my PR can't be used
I'm happy to share my findings so far
as we know the code working fine when lhs is a name, e.g.
(_init_p3 = /** @pureOrBreakMyCode */_apply_decs_2203_r(1, 2, 3));
but can't work when lhs is array pattern, e.g.
([_init_p3] = /** @pureOrBreakMyCode */_apply_decs_2203_r(1, 2, 3));
I think the possible cause is that when lhs is name, CC traverse the assignment as a whole
but when lhs is array pattern, CC respectively traverse left first, then traverse right which is a call. but CC doesn't check if a call is pure (I checked, the annotation is still attached to the callNode)
CLA signed
This might be a problem with the compiler's RemoveUnusedCode pass
If we can repro this with a unit test in RemoveUnusedCodeTest.java, we can confirm that RemoveUnusedCode is not supporting /** @pureOrBreakMyCode */ on LHS destructed assignment code pattern
@rahul-kamat this can be a potential unit test
/** @nocollapse */
const createComplexPureFunction1 = () => (x) => {
return x * 2;
};
/** @nocollapse */
const createComplexPureFunction2 = () => (x) => {
return { result: [x * 2] };
};
/** @nocollapse */
function complexPureFunction1(x) {
return (complexPureFunction1 = createComplexPureFunction1())(x)
}
/** @nocollapse */
function complexPureFunction2(x) {
return (complexPureFunction2 = createComplexPureFunction2())(x)
}
const a = /** @pureOrBreakMyCode */ complexPureFunction1(1);
let b;
({
result: [b],
} = /** @pureOrBreakMyCode */ complexPureFunction2(2));
in this example, only complexPureFunction1 gets removed completely, complexPureFunction2 still exist despite it has @pureOrBreakMyCode
I've spent some time looking into this, your latest example shows complexPureFunction1 completely getting removed but complexPureFunction2 is not removed by RemoveUnusedCode
I also added these 2 examples as unit tests for RemoveUnusedCode
example1
example1 shows code being removed, example2 does not. But in RemoveUnusedCode, the only thing removed from both examples is:
class A {
constructor(){
this.p3 = /** @pureOrBreakMyCode */_init_p3(this, "3");
}
}
so these 2 examples have the exact same behavior in RemoveUnusedCode
I'll need to spend more time figuring out what's going on