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

minifier should not skip callee in more cases #6047

Closed
kdy1 opened this issue Oct 5, 2022 · 2 comments · Fixed by #6053
Closed

minifier should not skip callee in more cases #6047

kdy1 opened this issue Oct 5, 2022 · 2 comments · Fixed by #6053
Assignees
Labels
Milestone

Comments

@kdy1
Copy link
Member

kdy1 commented Oct 5, 2022

Describe the bug

Related to #6044

If an expression has a side effect on the identifier in the callee position, this is not caught by the infection analysis.

We can fix this by aborting on reassignments to the callee, and this seems better.
(This resulted in a too much regression)

Input code

let foo = () => 1;

const obj = {
    get 0() {
        foo = () => 2;
        return 40;
    },
};
console.log(obj)

var c = obj[0];

console.log(foo(c));

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

Playground link

https://play.swc.rs/?version=1.3.4&code=H4sIAAAAAAAAA8tJLVFIy89XsFXQ0FSwtVMwtObiSs7PKy5RyE%2FKAopWcykAQTpQlQFQAYQHAsh6jKzhwkWpJaVFeQomBhChWh2uWmuwefk5qXo5%2BekaQFM1ubjKEosUkoH6gbxog1iolTAlQKM1kjU1rQG1tv8ymwAAAA%3D%3D&config=H4sIAAAAAAAAA22TQZLjIAxF7%2BJ1FlNZ9KIPMLs%2BA6WAcOjByIVEOq5U7j7CsXGc9M56%2FhLiS9y6b7bd560bITPm%2BsVTErh2nx3aAdjmMEp3UJkiD5HxfugEco9SJXz8czzq70jEuAgO3RBS8FMtZmkYMzLXb00qAybhpoOc6UdDyUWjE1FESG%2BxATYhCfaYt1RLMcLIaC6QW0Y9DXJgSjtUBJ0ZM41PNLkggZIWW5lDcMaSwwZCRivhgptEa6gksTbWOpmhw1Pp%2B2rgQ4kXiAWk1cLrbIOe2PLOFFiML4lf0K7TB3pcclEFbzJKyWnVfFNIOx%2F%2BIWqfEZgTDLhlztzrHPZK%2F4sqJK9DlKlRnfDWVcJeL2dC8O2GtWvMEja3Mrpisd7QbsUXuLsQB4cGvVe3WzL%2FBLHnrZhMI5JvoboGfpvBIzTrPu1pXYE3%2BFe7kudxLHwAOb8ynoYTxbcSA8qZ3BtWI4ReYdbtvY6vtCSHajO6px%2BFa9jWVIcvZGJ9bavPuhCaY%2FpIp7a%2B%2BigHSH1by3sF5EoFt9m8%2BbV%2BdPftda7rxF%2BLcC70H9%2FRCH8SBAAA

Expected behavior

It should not be inlined.

Actual behavior

No response

Version

main

Additional context

No response

@kdy1 kdy1 added the C-bug label Oct 5, 2022
@kdy1 kdy1 added this to the Planned milestone Oct 5, 2022
@kdy1 kdy1 self-assigned this Oct 5, 2022
@kdy1 kdy1 changed the title minifier should not inline member access in some case minifier should not skip callee in more cases Oct 5, 2022
@Austaras
Copy link
Member

Austaras commented Oct 5, 2022

A variant

let foo = () => 1;

const obj = new Proxy({}, {
    get () {
        foo = () => 2;
        return 40;
    },
});
console.log(obj)

var c = obj[0];

console.log(foo(c));

kdy1 added a commit to kdy1/swc that referenced this issue Oct 5, 2022
kdy1 added a commit to kdy1/swc that referenced this issue Oct 5, 2022
kdy1 added a commit to kdy1/swc that referenced this issue Oct 5, 2022
kdy1 added a commit to kdy1/swc that referenced this issue Oct 5, 2022
@kdy1 kdy1 closed this as completed in #6053 Oct 6, 2022
kdy1 added a commit that referenced this issue Oct 6, 2022
**Description:**

This PR fixes the callee issue by fixing the infection analyzer.

**Related issue:**

 - Closes #6047
@kdy1 kdy1 modified the milestones: Planned, v1.3.5 Oct 6, 2022
@swc-bot
Copy link
Collaborator

swc-bot commented Nov 5, 2022

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 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.