fix(es/minifier): Fix code for dropping unreachable statements

[kdy1]

Description:

We should preserve return in this case.

Related issue:

• Closes Whole if block is omitted in case throw after the return undefined #6405.

[kdy1]

swc-bump:

• swc_ecma_minifier

[jridgewell]

We have a before and after diff of:

+    if (!!val) {
         if (val.a?.b !== !0) throw Error('second');
         return val;
+    }

What I think this change is doing is keeping the return statement in the new output, which prevents us from optimizing if (!!val) if (val.a?.b !== !0) … into if (val.a?.b !== !0) …. But that optimization is still broken, right? That's not a valid compression, and I think we've missed the real bug.

[kdy1]

drop_unreachable_stmts is called for all kinds of statements, including stmts of BlockStmt.
And works only if there's an unreachable statement.

if (foo) {
   return;
   throw foo
}

will trigger this function, but

if (foo) {
   return;
}

will not.

It seems like I think it's a function called only for functions while writing this function initially.
But actually this is called for all kind of statements, and if so we should not drop return foo in

if (foo) {
   return foo;
   throw foo
}

return bar

[jridgewell]

Ok, so that's one bug that's fixed. But I'm still confused how, even if we were to drop the return statement, we end up also dropping the if (!!val) wrapper around the throw?

[kdy1]

Logs

I removed unrelated logs

Main:

  DEBUG  Dropped `undefined` from `return undefined`, kind: "change"
    at crates/swc_ecma_minifier/src/compress/pure/misc.rs:233
    in apply pure optimizer
    in optimize with pass: 1
    in compress ast
    in minify

  DEBUG  Dropping statements after a control keyword, kind: "change"
    at crates/swc_ecma_minifier/src/compress/pure/dead_code.rs:229
    in apply pure optimizer
    in optimize with pass: 1
    in compress ast
    in minify


  DEBUG  Converting empty block to empty statement, kind: "change"
    at crates/swc_ecma_minifier/src/compress/optimize/mod.rs:1228
    in visit_mut_stmt
    in visit_mut_if_stmt
    in visit_mut_stmt
    in handle_stmt_likes
    in visit_mut_block_stmt
    in visit_mut_arrow_expr
    in visit_mut_var_declarator
    in visit_mut_var_declarators
    in visit_mut_var_decl
    in visit_mut_decl
    in visit_mut_export_decl
    in handle_stmt_likes
    in apply full optimizer
    in optimize with pass: 1
    in compress ast
    in minify

  DEBUG  conditionals: `if (foo);` => `foo` , kind: "change"
    at crates/swc_ecma_minifier/src/compress/optimize/conditionals.rs:221
    in visit_mut_stmt
    in handle_stmt_likes
    in visit_mut_block_stmt
    in visit_mut_arrow_expr
    in visit_mut_var_declarator
    in visit_mut_var_declarators
    in visit_mut_var_decl
    in visit_mut_decl
    in visit_mut_export_decl
    in handle_stmt_likes
    in apply full optimizer
    in optimize with pass: 1
    in compress ast
    in minify

  DEBUG  Compressing boolean literal, kind: "change"
    at crates/swc_ecma_minifier/src/compress/optimize/mod.rs:578
    in visit_mut_bin_expr
    in visit_mut_if_stmt
    in visit_mut_stmt
    in handle_stmt_likes
    in visit_mut_block_stmt
    in visit_mut_arrow_expr
    in visit_mut_var_declarator
    in visit_mut_var_declarators
    in visit_mut_var_decl
    in visit_mut_decl
    in visit_mut_export_decl
    in handle_stmt_likes
    in apply full optimizer
    in optimize with pass: 1
    in compress ast
    in minify

  DEBUG  sequences: Compressing statements as a sequences, kind: "change"
    at crates/swc_ecma_minifier/src/compress/optimize/sequences.rs:157
    in handle_stmt_likes
    in visit_mut_block_stmt
    in visit_mut_arrow_expr
    in visit_mut_var_declarator
    in visit_mut_var_declarators
    in visit_mut_var_decl
    in visit_mut_decl
    in visit_mut_export_decl
    in handle_stmt_likes
    in apply full optimizer
    in optimize with pass: 1
    in compress ast
    in minify

Fixed:

DEBUG  Dropped `undefined` from `return undefined`, kind: "change"
    at crates/swc_ecma_minifier/src/compress/pure/misc.rs:233
    in apply pure optimizer
    in optimize with pass: 1
    in compress ast
    in minify

  DEBUG  Dropping statements after a control keyword, kind: "change"
    at crates/swc_ecma_minifier/src/compress/pure/dead_code.rs:229
    in apply pure optimizer
    in optimize with pass: 1
    in compress ast
    in minify

   DEBUG  if_return: Negating `foo` in `if (foo) return; bar()` to make it `if (!foo) bar()`, kind: "change"
    at crates/swc_ecma_minifier/src/compress/pure/if_return.rs:89
    in postcompress
    in minify

[kdy1]

So...

previously

export const fn = () => {
    let val;

    if (!val) {
        return undefined;
        // works as expected if comment out below line
        throw new Error('first');
    }

    if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
        throw Error('second');
    }
    return val;
}

=>

export const fn = () => {
    let val;

    if (!val) {
    }

    if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
        throw Error('second');
    }
    return val;
}

=>

export const fn = () => {
    let val;

    if (!val) ;

    if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
        throw Error('second');
    }
    return val;
}

=>

export const fn = () => {
    let val;

    if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
        throw Error('second');
    }
    return val;
}

[jridgewell]

Got it, it's because it's the first if statement that has the return dropped.

[kdy1]

New version:

export const fn = () => {
    let val;

    if (!val) {
        return;
    }

    if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
        throw Error('second');
    }
    return val;
}

=> (negation from if_return)

export const fn = () => {
    let val;

    if (val) {
    // condition is negated to remove return token

        if (val.a?.b !== true) { // Uncaught TypeError: Cannot read properties of undefined (reading 'a')
            throw Error('second');
        }
        return val;
     
    }

}

[jridgewell]

Got it, it's because it's the first if statement that has the return dropped.

[swc-bot]

Automated review comment generated by auto-rebase script