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

Mangle function name duplicate with the parameter name #6819

Closed
noyobo opened this issue Jan 16, 2023 · 12 comments · Fixed by #6821
Closed

Mangle function name duplicate with the parameter name #6819

noyobo opened this issue Jan 16, 2023 · 12 comments · Fixed by #6821
Assignees
Labels
Milestone

Comments

@noyobo
Copy link

noyobo commented Jan 16, 2023

Describe the bug

output code:

export function __CREATE_EXP__() {
    return function n(...n) { // n is duplicate
        return value;
    };
}

run in Safari throw error:

SyntaxError: Duplicate parameter 'e2' not allowed in function with a rest parameter.

image

But it works in chrome...

Input code

export function __CREATE_EXP__() {
  return function EXP(...args) {
      return value;
  };
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false
    },
    "externalHelpers": true,
    "target": "es2015",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": {
        "toplevel": false,
        "keep_classnames": false,
        "keep_fnames": false,
        "keep_private_props": false,
        "ie8": false,
        "safari10": false
      }
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true
}

Playground link

https://play.swc.rs/?version=1.3.26&code=H4sIAAAAAAAAA0utKMgvKlFIK81LLsnMz1OIj3cOcnUMcY13jQiIj9fQVKjmUlAoSi0pLcpDKALKaejp6SUWpRdDFIAAVFFZYk5pqjVQrNaaq5YLAMflGI9fAAAA&config=H4sIAAAAAAAAA22PQQrCQAxF75J1Fyoo0hO48QwS6q%2BMTmdCEqVFenenUi2Cq4T%2F85O8J12tofpJwmrQqbMhOfdUkw8CazSIU0VuRWo5GsaK0Ds0cTwgCtSodr2jzLBe4CUJ26zW25KKORvmXEVdSKEdphtN7kRhtlicLhGT5VkiHohf6wbIqYlslriD%2FertH000PNhRapbFCth%2Fe%2BOWNaxXH6KxMHX5fJ8%2FKNxviB2Ny9OfNXacByfm8QVyauLJQAEAAA%3D%3D

Expected behavior

export function __CREATE_EXP__() {
    return function EXP(...n) { //  EXP name ? 
        return value;
    };
}

Actual behavior

No response

Version

1.3.26

Additional context

No response

@noyobo noyobo added the C-bug label Jan 16, 2023
@noyobo
Copy link
Author

noyobo commented Jan 16, 2023

Related issue?#6730

@kdy1 kdy1 modified the milestones: Blocked by type checker, Planned Jan 16, 2023
@kdy1 kdy1 self-assigned this Jan 16, 2023
@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

I didn't know that a rest parameter behaves differently than a normal parameter...

@noyobo
Copy link
Author

noyobo commented Jan 16, 2023

In this file and the closure scope, EXP is just a function display name, which is convenient for me to debug in devtools, and there is no reference.

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

@iheyunfei Do you have any idea to fix this?
I'm now thinking about the way, but my solution requires a strange scoping trick.

@hyf0
Copy link
Contributor

hyf0 commented Jan 16, 2023


edited:

I just check it, it's not right. Let me do some digging.


Yeah. A simple way is adding a line like

fn visit_fn_decl(&mut self, f: &FnDecl) {
        self.add_decl(f.ident.to_id(), true);

        self.with_fn_scope(|v| {
+            v.add_usage(f.ident.to_id());
            f.function.decorators.visit_with(v);
            f.function.params.visit_with(v);
            // WARN: Option<BlockStmt>::visit_mut_children_wth
            // is not same with BlockStmt::visit_mut_children_wth
            v.visit_fn_body_within_same_scope(&f.function.body);
        })
    }

in

fn visit_fn_decl(&mut self, f: &FnDecl) {
self.add_decl(f.ident.to_id(), true);
self.with_fn_scope(|v| {
f.function.decorators.visit_with(v);
f.function.params.visit_with(v);
// WARN: Option<BlockStmt>::visit_mut_children_wth
// is not same with BlockStmt::visit_mut_children_wth
v.visit_fn_body_within_same_scope(&f.function.body);
})
}

FnExpr should also have this issue.

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

Thank you! I'll try

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

Oh, I think it will regress all cases.

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

I'll check for rest and apply your patch 😄

@hyf0
Copy link
Contributor

hyf0 commented Jan 16, 2023

The idea is correct in general. But the code I provided seems wrong. We need to create fake usages for names that could be conflicting with others.

image

I also look out Terser, the outputs are
image
image

edited:

I just check it, it's not right. Let me do some digging.

Yeah. A simple way is adding a line like

fn visit_fn_decl(&mut self, f: &FnDecl) {
        self.add_decl(f.ident.to_id(), true);

        self.with_fn_scope(|v| {
+            v.add_usage(f.ident.to_id());
            f.function.decorators.visit_with(v);
            f.function.params.visit_with(v);
            // WARN: Option<BlockStmt>::visit_mut_children_wth
            // is not same with BlockStmt::visit_mut_children_wth
            v.visit_fn_body_within_same_scope(&f.function.body);
        })
    }

in

fn visit_fn_decl(&mut self, f: &FnDecl) {
self.add_decl(f.ident.to_id(), true);
self.with_fn_scope(|v| {
f.function.decorators.visit_with(v);
f.function.params.visit_with(v);
// WARN: Option<BlockStmt>::visit_mut_children_wth
// is not same with BlockStmt::visit_mut_children_wth
v.visit_fn_body_within_same_scope(&f.function.body);
})
}

FnExpr should also have this issue.

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

It was correct for FnExpr. See: #6821

@kdy1
Copy link
Member

kdy1 commented Jan 16, 2023

Thank you! I fixed it by creating a fake usage

kdy1 added a commit that referenced this issue Jan 16, 2023
@kdy1 kdy1 modified the milestones: Planned, v1.3.27 Jan 17, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Feb 16, 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 Feb 16, 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.

4 participants