`@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](https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540formatting%2520pretty_print%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F**%2520%2540noinline%2520*%252F%250Aconst%2520applyDecs2203RFactory%2520%253D%2520()%2520%253D%253E%2520%257B%2520return%2520()%2520%253D%253E%2520%257B%257D%2520%257D%250A%252F**%2520%2540noinline%2520*%252F%250Aconst%2520_apply_decs_2203_r%2520%253D%2520applyDecs2203RFactory()%250Avar%2520_init_p3%253B%250A%250Aclass%2520A%2520%257B%250A%2520%2520%2520%2520constructor()%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520this.p3%2520%253D%2520%252F**%2520%2540pureOrBreakMyCode%2520*%252F_init_p3(this%252C%2520%25223%2522)%253B%250A%2520%2520%2520%2520%257D%250A%257D%250A(_init_p3%2520%253D%2520%252F**%2520%2540pureOrBreakMyCode%2520*%252F_apply_decs_2203_r(1%252C%25202%252C%25203))%253B)

[example2](https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540formatting%2520pretty_print%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F**%2520%2540noinline%2520*%252F%250Aconst%2520applyDecs2203RFactory%2520%253D%2520()%2520%253D%253E%2520%257B%2520return%2520()%2520%253D%253E%2520%257B%257D%2520%257D%250A%252F**%2520%2540noinline%2520*%252F%250Aconst%2520_apply_decs_2203_r%2520%253D%2520applyDecs2203RFactory()%250Avar%2520_init_p3%253B%250A%250Aclass%2520A%2520%257B%250A%2520%2520%2520%2520constructor()%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520this.p3%2520%253D%2520%252F**%2520%2540pureOrBreakMyCode%2520*%252F_init_p3(this%252C%2520%25223%2522)%253B%250A%2520%2520%2520%2520%257D%250A%257D%250A(%257B%2520e%253A%2520_init_p3%2520%257D%2520%253D%2520%252F**%2520%2540pureOrBreakMyCode%2520*%252F_apply_decs_2203_r(1%252C%25202%252C%25203))%253B)

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