Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compression misplaces function call when variable storing return value is used for logical assignment #8119

Closed
augustjk opened this issue Oct 13, 2023 · 3 comments · Fixed by #8128
Assignees
Labels
Milestone

Comments

@augustjk
Copy link

augustjk commented Oct 13, 2023

Describe the bug

When a return value of a function is assigned to a let variable, and that variable is used behind a logical assignment, compression tries to remove that intermediate variable and move the function call directly after the logical assignment.

This changes the logic such that before compilation the function would have always been called, but after compilation it's moved behind the logical assignment and will be called conditionally.

Since the function can have a side-effect that should always run, this is incorrect behavior.

Input code

const myArr = [];
// function with side effect
function foo(arr) {
    arr.push('foo');
    return 'foo';
}
let a;

if (Math.random() > 0.5) {
    a = true;
}

// the function call below should always run
// regardless of whether `a` is `undefined`
let b = foo(myArr);

// const seems to keep this line here instead of
// moving it behind the logitcal nullish assignment
// const b = foo(myArr);

a ??= b;

console.log(a);
console.log(myArr);

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": 2
      },
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true
}

Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.3.74&code=H4sIAAAAAAAAA2WQTU7DMBCF9z7F2zXZpGxYRaXiAJygQsoknsQWjl35h6hC3J1xJFokvBrPzHvvs6fgU8Z6e40RJ1zee3U8Yi5%2ByjZ4bDYbJKtZ3VtzCA3F2OJLQY6U3bUk0xxkcGj7vRk5l%2Bixt3r1rRxnUK%2BUndG8UTZdJK%2FD2rR4wVP3fPcSghwLV0nFyIYfKBM5h5Fd2JBMKE6D3Ea3hFh8XY68UNSOU0KYsRkWdcRAA2zCULzm2XrWww4zSlJ9yP5uga4G0%2F4TiXlNyAEfzFchELETHcSMYWWDSUtAFazh0%2FoFVuzYWK93XhcWm4UVvjhnkwGlZBe%2Fss%2BPkH%2FxhPP5hFGquhAcd%2BLTkIz%2B3n%2FXfwDJh74ktAEAAA%3D%3D&config=H4sIAAAAAAAAA32UO3LjMAyGe5%2FCozrFjostcoDtcgYOTYIyvXxoCNCxJuO7L0TJjjeG1En48AMkAOJrt993ZzTd%2B%2F6LP%2Fln0AWhPP7ZgmMifWVLByZqNMUP1L3d6Rkn5HRAaKbbTDrSpQdqKjz8OhwWRRdyRrgrFlv0ybvxOafJcSiA%2BGRjK4esERLh%2F%2FqFlfw5ASr12X7MOYBOG0RpVD4R9FCkwCaHoAcEddFFiDKdVBePWUoxwUpg1VDyIPJkPfmcOOcrtaCtMtmCgHwBQ%2F4CkoxzsSwhX0%2B4T8MWjrXvW59%2FqOGiQ9Uk5IRrawmfVoh6yh5JuZqkEs5wpQYzXIr7U%2BmdKkC1pFfdOfu00pO%2FAFyBoBGTjiDFbR6O52lN7TaVPjkeWRoFzvMt3TJBz0VV3juhslNloJCXulnAVgNTZY10nAWvlA%2B9BQXO8awIofHTkzlJSWkcIDsBcH%2B1k6ZqBurxClf49CA28B%2B%2BJckDtnhETad1imM85rCRIAKdst1w4FZQXseFt8R1WOc1WeDRACu6VGzgdQnwA6CsQtuXL7PBz4Mjqj7ko7gmBp7z1sTDYrs9lnPUqf9eAvN%2B3i0OXcy2Nrhs%2Fqnp877%2B3X073Vfz4zadx4%2B7cjrJ7vYPdLfRTEUGAAA%3D

SWC Info output

No response

Expected behavior

In above input, the output should keep b = foo(myArr) like how it does when it's declared with const instead of let.

Actual behavior

No response

Version

1.3.74

Additional context

No response

@kdy1
Copy link
Member

kdy1 commented Oct 16, 2023

Seems like this occurs only if someone tries to minify using transform() instead of minify().

@kdy1
Copy link
Member

kdy1 commented Oct 16, 2023

Strange thing is that

const myArr = [];
        function foo(arr) {
            arr.push('foo');
            return 'foo';
        }
        let a;
        if (Math.random() > ) {
            a = true;
        }
        let b = foo(myArr);
        a ??= b;
        console.log(a);
        console.log(myArr);

is Start of the input when it's invoked via transform(), but minifier does not break it if I feed the Start to it directly.

So... my guess is that it's related to span hygiene.

kdy1 added a commit that referenced this issue Oct 18, 2023
@kdy1 kdy1 modified the milestones: Planned, v1.3.94 Oct 21, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Nov 20, 2023

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.

@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

3 participants