fix corner case in dead_code

[alexlamsl]

fixes #3552

[kzc]

@alexlamsl Off topic... I've never known the difference between the compress options dead_code and unused. At one point before either of us were involved with the project I think there might have been a distinction between them, but the delegation of concerns between the two options seem rather arbitrary. Some dead/unused code logs warnings about code removal and others do not. Would it make sense to unify these options and one just an alias of the other for backwards compatibility?

[alexlamsl]

I tried at one point but gave up after discovering the mass of tests I need to update.

And while we are on dead_code & unused — what about side_effects?

[kzc]

side_effects seems reasonable as implemented to me, although there are many one-off back door mini-side_effects-like implementations scattered throughout the code. Is that what you mean? side_effects does take a lot of CPU, so it's worth gating I think. Admittedly half the compress options do not work without it, but the same can be said for evaluate.

The one thing that I regret not adding would be a compress option annotations specifically to gate pure annotations rather than relying on the large club that is side_effects.

[kzc]

For the most part I think the compress options are pretty orthogonal in their responsibilities and are reasonable. I think it's better to err on the side of providing more configurability rather than less. That way when there's a bug it's trivial for users to set an option to work around it. It's also useful for debugging.

[alexlamsl]

Yes, that is why I ended up leaving dead_code as is in the end - it makes hunting bugs down easier. Although unused interacts with a lot of other optimisations, its logic is actually fairly localised (mostly inside drop_unused()), whereas dead_code is scattered around places.

[alexlamsl]

The one thing that I regret not adding would be a compress option annotations specifically to gate pure annotations rather than relying on the large club that is side_effects.

Given that it requires modifying the parser to handle said annotation (for optimal performance), I guess that extra flag wouldn't exactly help much with debugging.

That said, if there is a valid usage scenario for explicitly disabling @__PURE__ handling, I can easily introduce a flag like that.

[kzc]

Babel has a bug where it incorrectly generates pure annotations with respect to parentheses in some cases. When the final minified code did not operate correctly it was not readily apparent whether there was a Babel bug or a minifier bug. An annotation option would have been marginally useful to debug that. Instead I just used sed to change the comments in question. No big deal for me. But come to think of it judging by the poor quality of most bug reports, the average JS coder wouldn't have a clue how to debug it with or without such an option anyway.

[alexlamsl]

Next time just short-circuit this out, i.e. simply return; or comment out call.pure = ...
https://github.com/mishoo/UglifyJS2/blob/0f4cfa877a9f14a75b219c04947ce5984772d8ba/lib/parse.js#L1423-L1434

But come to think of it judging by the poor quality of most bug reports, the average JS coder wouldn't have a clue how to debug it with or without such an option anyway.

More fun for us 😉
_{^{as long as the conversation stays civil}}

[kzc]

Thanks. The option wasn't intended for me so much as for users. I know my way around the code pretty well. ;-)