Skip to content

Commit

Permalink
fix #856: remove "use asm" directives
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 20, 2021
1 parent 6d6d306 commit 39ac908
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@

However, the CommonJS `module` and `exports` variables that are arguments to the closure previously weren't considered to be used in this scenario, meaning they may be omitted as dead code for size reasons. This could cause code inside `eval` to behave incorrectly. Now use of direct `eval` automatically counts as a use of both `module` and `exports` so these variables should now always be present in this case.

* Always remove all `"use asm"` directives ([#856](https://github.com/evanw/esbuild/issues/856))

The asm.js subset of JavaScript has complicated validation rules that are triggered by this directive. The parser and code generator in esbuild was not designed with asm.js in mind and round-tripping asm.js code through esbuild will very likely cause it to no longer validate as asm.js. When this happens, V8 prints a warning and people don't like seeing the warning. The warning looks like this:

```
(node:58335) V8: example.js:3 Invalid asm.js: Unexpected token
(Use `node --trace-warnings ...` to show where the warning was created)
```
I am deliberately not attempting to preserve the validity of asm.js code because it's a complicated legacy format and it's obsolete now that WebAssembly exists. By removing all `"use asm"` directives, the code will just become normal JavaScript and work fine without generating a warning.
## 0.8.49
* Work around a problem with `pnpm` and `NODE_PATH` ([#816](https://github.com/evanw/esbuild/issues/816))
Expand Down
19 changes: 18 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6132,9 +6132,26 @@ func (p *parser) parseStmtsUpTo(end js_lexer.T, opts parseStmtOpts) []js_ast.Stm
stmt.Data = &js_ast.SDirective{Value: str.Value, LegacyOctalLoc: str.LegacyOctalLoc}
isDirectivePrologue = true

// Track "use strict" directives
if js_lexer.UTF16EqualsString(str.Value, "use strict") {
// Track "use strict" directives
p.currentScope.StrictMode = js_ast.ExplicitStrictMode
} else if js_lexer.UTF16EqualsString(str.Value, "use asm") {
// Deliberately remove "use asm" directives. The asm.js subset of
// JavaScript has complicated validation rules that are triggered
// by this directive. This parser is not designed with asm.js in
// mind and round-tripping asm.js code through esbuild will very
// likely cause it to no longer validate as asm.js. When this
// happens, V8 prints a warning and people don't like seeing the
// warning.
//
// We deliberately do not attempt to preserve the validity of
// asm.js code because it's a complicated legacy format and it's
// obsolete now that WebAssembly exists. By removing this directive
// it will just become normal JavaScript, which will work fine and
// won't generate a warning (but will run slower). We don't generate
// a warning ourselves in this case because there isn't necessarily
// anything easy and actionable that the user can do to fix this.
stmt.Data = &js_ast.SEmpty{}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,24 @@
)
}

// Make sure that the "asm.js" directive is removed
tests.push(
test(['in.js', '--outfile=node.js'], {
'in.js': `
function foo() { 'use asm'; eval("/* not asm.js */") }
let emitWarning = process.emitWarning
let failed = false
try {
process.emitWarning = () => failed = true
foo()
} finally {
process.emitWarning = emitWarning
}
if (failed) throw 'fail'
`,
}),
)

let simpleCyclicImportTestCase542 = {
'in.js': `
import {Test} from './lib';
Expand Down

0 comments on commit 39ac908

Please sign in to comment.