Skip to content

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 81 commits into from
Apr 6, 2021
Merged

Conversation

nevkontakte
Copy link
Member

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 🥳

nevkontakte and others added 30 commits February 28, 2021 16:22
  - 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.
`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.
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.
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).
nevkontakte and others added 22 commits March 29, 2021 09:48
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.
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()
@nevkontakte nevkontakte requested a review from flimzy April 6, 2021 09:43
@paralin
Copy link
Contributor

paralin commented Apr 10, 2021

Awesome work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go 1.16 standard library support
4 participants