-
Notifications
You must be signed in to change notification settings - Fork 570
Add support for Go 1.16.3 standard library #1015
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update CicleCI to use Go 1.16. - Update version check to allow running against 1.16 GOROOT.
Support compiling Go 1.16 `runtime` with GopherJS. The vast majority of the code (even in errors.go) is not used by GopherJS, so starting with this commit the package is entirely defined by our own implementation in natives. As a bonus, this allowed to drop a few unexported symbols that existed purely to let other unused functions to compile.
Go 1.13 introduced two more low-level fields that would've been referenced by essentially dead code, so I decided to replace original pool.go entirely. As a byproduct, this might yield a tiny decrease in output size (assuming DCE hasn't been pruning the unexported methods away already).
math.FMA() implementation introduced a non-exported function zero(), which conflicted with out own variable. Resolved the conflict by renaming our variable. Original commit by @visualfc: goplusjs/gopherjs@ad4d281
Looks like a few new functions were added to the package, with a few new constants. Neither of them are really relevant in the JS context and arguably we shouldn't be compiling this code in the first place (see issue #991), but this lets the package compile for now.
I'm not sure of the role of the seq field plays outside of the time package, but I'll add it there. In the upstream code the runtimeTimer struct seems to replicate (and require to be compatible with) runtime.timer struct, however in GopherJS it is clearly not and it doesn't seem to be a problem. Similarly, in the upstream startTimer, stopTimer, resetTimer and modTimer are actually implemented in the `runtime` package, but since we already re-implemented two of them here, I'll just keep this package self-contained.
Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
This package is meant to be a cut-down version of the `reflect` package, which is used by packages like `errors` to avoid a cyclic dependency with `reflect`. This code is based on goplusjs/gopherjs@8e608e5. The natives code of the reflectlite package is a subset of its counterpart from reflect. Unfortunately, this requires us to duplicate a substantial chunk of code with all the maintenance problems that stem from it. I have considered two alternative approaches, none of which worked out: - Automatically rewrite imports from `internal/reflectlite` to `reflect`. Unfortunately, this creates cyclic dependencies and causes compilation to fail. - Redefine types in reflectlite as aliases of their reflect counterparts. This both causes a cyclic dependency and creates a divergence between GopherJS and Go standard library. TinyGo seems to be doing that somehow, but I suspect that they are able to handle cyclic dependencies. As a bottom line, duplicating a subset of reflect code is the lesser evil that is available to us. In the long term we could try and converge our reflect implementation with the upstream, so that this is no longer our problem. Co-authored-by: visualfc <visualfc@gmail.com>
Add `internal/reflectlite` package support.
The warning: > compiler/utils.go:220:12: conversion from int to string yields a > string of one rune, not a string of digits (did you mean > fmt.Sprint(x)?) Suppress the warning by converting to the rune explicitly.
There were two major issues with the package: - os.RemoveAll() implementation on linux was using Fstatat syscall, which doesn't work with the node-syscall extension (see #993). - runtime.Callers() is not implemented by GopherJS, which causes testing package to report source code locations according to the JS output file rather than the original Go code. I had to skip a bunch of tests because of that, but at least most other tests can be tracked now.
Support `testing` package with Go 1.16 and re-enable most of its tests.
`go generate ...`
`FuncForPC()` returns function/file/line metadata for any given code position (effectively a memory address). However, since GopherJS runtime doesn't have access to raw addresses, we emulate this by assigning virtual position counters for call places seen by Caller() and use them to retrieve the original function information. This is a more limited implementation that the upstream, but I expect it to work for most of the real-life cases, and it fixes a few test failures in the `reflect` package. With this it should be fairly easy to bring back proper support for `testing.T.Helper()` function. Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
Previously we could return anything that may or may not have matched the required function signature, causing hard to debug failures in the code. This change replicates the upstream Go behavior.
In the upstream it is implemented in `runtime` and exported into `reflect` via a go:linkname directive, but GopherJS doesn't support that and we already define a local typeOffList in `reflect` as a substitute. Also delete it from reflectlite, since it isn't actually used there. Co-authored-by: visualfc <visualfc@gmail.com>
It has been introduced all the way back in Go 1.10, but GopherJS never properly supported it. The new implementation seems to pass most of the tests, which is an improvement. There are a couple of tests that are still failing, but fixing that seems like a harder task. The code is based on goplusjs/gopherjs@a293d52. Co-authored-by: visualfc <visualfc@gmail.com>
This reflects a change in the upstream intended to make use of terms "value" and "element" more consistent in the context of maps.
This commit is based on goplusjs/gopherjs@f4ae589 and goplusjs/gopherjs@143d71b. Co-authored-by: visualvc <visualfc@gmail.com>
The original test was using a signalling NaN, but JavaScript appears to coerce all NaNs to quiet ones. I found no mention of this behavior in the spec, but I suspect it is just the behavior JavaScript historically implemented when dealing with NaNs. Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
Several `reflect` tests rely on it to execute correctly.
`go generate ./...`
Support Go 1.16 `reflect` package.
Because certain built-in function may not be translated into callable expressions, GopherJS wraps them into a proxy lambda function when necessary. Prior to this change it was incorrectly inferring argument types for this proxy function, which led to a possible double implicit type conversion and a runtime error in the generated code. With this change, the types are assigned in accordance with what the deferred function expects, and therefore no double conversion will happen. This change allows TesSliceNoCycle in `encoding/json` to pass under Go 1.16.
Avoid double implicit type conversion on deferred built-in arguments.
GopherJS runtime doesn't provide access to memory allocation counters.
This is a new test involving system root certificates, which GopherJS doesn't support.
GopherJS currently doesn't support file embedding (see issue #997).
If required modules are not available (e.g. in a browser), we fall back to console.log(). This also allows us to pass fixedbugs/issue22326.go test in Go 1.16.
These tests cover nuances of interaction between unsafe and GC, which are not applicable to the JavaScript runtime. Note: fixedbugs/issue24491a.go seems to panic where I don't think it should, is likely related to issue #1001.
These tests rely on features GopherJS currently doesn't support.
Upon a cursory look, the test seems to panic when it is expected to, but the panic message is formatted differently, hence the test failure.
This mirrors the upstream golang/go#31546 and allows `fixedbugs/issue31546.go` to pass.
This test manifested a different issue in GopherJS compiler, which requires more research than I currently have time for. See #1007 for details.
Tests use an unsupported compiler flag and target aspects of `unsafe` package GopherJS doesn't support.
This addresses the point of the test, but we still have to skip it because printed output is formatted slightly differently.
$callDeferred() uses `throw null` to interrupt normal execution when called from a panic(). However, it simply returns when called orderly at the end of a function. A tricky interaction may happen when one of the deferred function blocks on a channel or mutex. The goroutine will get paused and when it unblocks, deferrals will continue to execute via $callDeferred() at the end of the function. Under that circumstance `throw null` trick is unnecessary since the main body of the function is outside of the call stack anyway. This fix allows fixedbugs/issue40629.go test to pass successfully.
3f703aa enabled source map support in the CI, so this test is now able to pass.
These tests take quite a while to complete, so build able to focus on our own tests is a time saver during development.
Fix or skip newly added Go compiler tests in 1.16
- Added packages, which were missing from the list entirely. - Added a few packages which seem to actually pass all tests. - Removed packages which pass all tests or have no tests from the test exclusion list.
Also updated the readme to reflect 1.16 support.
Documentation overhaul
Go 1.16.3 added yet another test in the `testing` package, which relied upon these functions, but since most of the support already existed, I decided to complete it instead of skipping yet another test. As a result, I was able to re-enable several tests in `io` and `testing` packages.
Update to Go 1.16.3 and add full support for runtime.Callers()
flimzy
approved these changes
Apr 6, 2021
This was referenced Apr 9, 2021
Closed
Awesome work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merging go1.16-stdlib into the master to introduce Go 1.16.3 support into GopherJS
We are skipping support for Go 1.13-1.15 and jumping straight to 1.16, since this is the current stable. Unfortunately, we don't have resources necessary to make separate releases for all intermediate Go versions that occurred since the previous GopherJS release.
Note: to the best of my knowledge, this should work with any 1.16.x release to date, but 1.16.3 is what we are testing against.
Fixes #989 🥳