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.

Sign up for GitHub

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

Jump to bottom

misc/wasm: wasm_exec.js does not enable strict mode #47116

Closed
rhysd opened this issue Jul 10, 2021 · 7 comments
Closed

misc/wasm: wasm_exec.js does not enable strict mode #47116

rhysd opened this issue Jul 10, 2021 · 7 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@rhysd
Copy link
Contributor

rhysd commented Jul 10, 2021 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.16.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/rhysd/Library/Caches/go-build"
GOENV="/Users/rhysd/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/rhysd/.go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/rhysd/.go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.16.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.16.5/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/rhysd/.go/src/github.com/rhysd/actionlint/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/98/2tcw8j4157578xpvbqfrwdyr0000gn/T/go-build2360158005=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I'm running Wasm file with browsers which was compiled from Go source in my project. It works fine, but when I read misc/exec_wasm.js, I noticed that it did not enable strict mode. Enabling strict mode means:

What did you expect to see?

The MDN document says:

Strict mode makes several changes to normal JavaScript semantics:

  1. Eliminates some JavaScript silent errors by changing them to throw errors.
  2. Fixes mistakes that make it difficult for JavaScript engines to perform optimizations: strict mode code can sometimes be made to run faster than identical code that's not strict mode.
  3. Prohibits some syntax likely to be defined in future versions of ECMAScript.

Due to historical reasons, browsers don't enable strict mode by default, but it should be enabled to avoid several JS pitfalls. For example, TypeScript recommends enabling strict mode.

If this change looks good to maintainers, I could submit a patch for this following the strict mode migration guide.

What did you see instead?

misc/exec_wasm.js did not enable strict mode.
@seankhliao seankhliao added arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 12, 2021
@seankhliao
Copy link
Member

cc @neelance @cherrymui

@neelance
Copy link
Member

Strict mode should probably be fine. 👍

@rhysd
Copy link
Contributor Author

rhysd commented Jul 12, 2021

Can I try to make changes for this issue following the guideline?

@neelance
Copy link
Member

Sure :)

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/334269 mentions this issue: misc/wasm: enable ECMAScript strict mode

@rhysd
Copy link
Contributor Author

rhysd commented Jul 13, 2021

@neelance I made change for fixing this issue. Would you take a look?

@rhysd
Copy link
Contributor Author

rhysd commented Jul 27, 2021

@neelance How can we proceed the review? If I need to do something, please let me know. I'll do it.

@golang golang locked and limited conversation to collaborators Sep 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@neelance @rhysd @gopherbot @seankhliao