@pureOrBreakMyCode can't handle when left-hand side is array pattern

[jzhan-canva]

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));

[rishipal]

I'm seeing that the LHS is eliminated even without the @pureOrBreakMyCode annotation. Why do you think this is specific to the annotation?

[jzhan-canva]

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

[rishipal]

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.

[jzhan-canva]

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)

[jzhan-canva]

CLA signed

[rahul-kamat]

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

[jzhan-canva]

@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

[rahul-kamat]

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

example2

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