Minifier should check type of assignment target before merging assignments

[hyp3rflow]

Describe the bug

Currently in replace_seq_assignment step, we don't check the type of target.
however, if the type of target differs, the total count of actual operations matters.

Input code

let a = '';
console.log((a += 1, a += 2))

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": false
    },
    "target": "es2022",
    "loose": false,
    "minify": {
      "compress": {
        "arguments": false,
        "arrows": true,
        "booleans": true,
        "booleans_as_integers": false,
        "collapse_vars": true,
        "comparisons": true,
        "computed_props": true,
        "conditionals": true,
        "dead_code": true,
        "directives": true,
        "drop_console": false,
        "drop_debugger": true,
        "evaluate": true,
        "expression": false,
        "hoist_funs": false,
        "hoist_props": true,
        "hoist_vars": false,
        "if_return": true,
        "join_vars": true,
        "keep_classnames": false,
        "keep_fargs": true,
        "keep_fnames": false,
        "keep_infinity": false,
        "loops": true,
        "negate_iife": true,
        "properties": true,
        "reduce_funcs": false,
        "reduce_vars": false,
        "side_effects": true,
        "switches": true,
        "typeofs": true,
        "unsafe": false,
        "unsafe_arrows": false,
        "unsafe_comps": false,
        "unsafe_Function": false,
        "unsafe_math": false,
        "unsafe_symbols": false,
        "unsafe_methods": false,
        "unsafe_proto": false,
        "unsafe_regexp": false,
        "unsafe_undefined": false,
        "unused": true,
        "const_to_let": true,
        "pristine_globals": true,
        "passes": 3
      },
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true
}

Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.4.6&code=H4sIAAAAAAAAA8tJLVFIVLBVUFe35krOzyvOz0nVy8lP19BIVNC2VTDUUQDTRpqaAAJDHUQpAAAA&config=H4sIAAAAAAAAA32UO5LbMAyGe5%2FCozpF4iJFDrBdzsChSVCmlw8NAXqt2fHdA1Gy11lD6iR8%2BAESAPG52%2B%2B7M5ruz%2F6TP%2Fln0AWhPP7ZgmMifWVLByZqNMUP1P240zNOyOmA0Ey3mXSkSw%2FUVHj4eTgsii7kjHBXLLbok3fjc06T41AA8cnGVg5ZIyTC%2F%2FULK%2FljAlTqs%2F2YcwCdNojSqHwi6KFIgU0OQQ8I6qKLEGU6qS4es5RigpXAqqHkQeTJevI5cc5XakFbZbIFAfkChvwFJBnnYllCvp5wn4YtHGvftz5%2FU8NFh6pJyAnX1hI%2BrRD1lD2ScjVJJZzhSg1muBT3u9I7VYBqSa%2B6c%2FZppSfvAFyBoBGTjiDFbR6O52lN7TaVPjkeWRoFzvMt3TJBz0VV3juhslNloJCXulnAVgNTZY10nAWvlA%2B9BQXO8awIofHDkzlJSWkcIDsBcH%2B1k6ZqBurxClf49CA28BvfkuQBWzyiptM6xTEec9hIEIFO2W44cCsor%2BPCW%2BI6rPOaLPBogBVdKjbwugT4AVBWoe3Ll9ng58ERVR%2FyUVwTA895a%2BKvxXZ7LOeoU%2F%2B1BOb9vFscuphtbXDZ%2FFPT5339u%2Ftyuq%2Fmx206j3%2Fvyukku9s%2FdoKzJEUGAAA%3D

SWC Info output

No response

Expected behavior

let a = '';
console.log('12')

or don't minify it

Actual behavior

let a = '';
console.log((a += 3))

which results '3'

Version

1.4.6

Additional context

No response

[CPunisher]

@kdy1 🤔 Since it's non-trivial to track the changes of types of vars, it seems impossible to do the check.

[kdy1]

@CPunisher Does this still happen? I made similar conclusion and I disabled such operations with #9575

[kdy1]

@CPunisher, are you also on SWC Discord? I have a question for you, but I want to ask it privately.

[CPunisher]

@CPunisher Does this still happen? I made similar conclusion and I disabled such operations with #9575

Yes, it still happens. This bug is caused by not considering the type of the var, while that pr considers the types of right operands in the assignment sequence.

Edit by maintainers: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

[kdy1]

I'm under the impression that we should disable optimization of += if any of the operands has an unknown type. What do you think?

[CPunisher]

Let's discuss everything here.

Is collecting initial types enough? I think it can be useful for const but not for others

No, it's not enough. I realize that the usage analyzer is flow-insensitive, so it's hard to know the accurate type of a variable and I get stuck.

I'm under the impression that we should disable optimization of += if any of the operands has an unknown type. What do you think?

But is it possible to know whether the operand has an unknown type?

[kdy1]

But is it possible to know whether the operand has an unknown type?

Yeap, there's a get_type in Expr. Although it does not analyze the type of variables, it's enough for guaranteeing correctness. We may add some logic for analyzing the types of the variable in the future.

[CPunisher]

But is it possible to know whether the operand has an unknown type?

Yeap, there's a get_type in Expr. Although it does not analyze the type of variables, it's enough for guaranteeing correctness. We may add some logic for analyzing the types of the variable in the future.

So in this case:

let a = 0;
// a = ""; Maybe it changes
console.log((a += 1, a += 2))

The only thing we know about a is that its initial type is number. When visiting a += 1, a += 2, we can't analyze the type of variable a. Does this mean we should disable the optimization in this case? If so, we may disable all the optimization when either the left or the right part of assignment contains variables. This almost means the optimization is useless.

[kdy1]

In the sequential optimizer a: Mergable will be let a = 0 and b: Mergable will be a += 1 so I think it will be still optimized.

[swc-bot]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.