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.

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

Add Go 1.18 support #1116

Merged
merged 51 commits into from Aug 7, 2022
Merged

Add Go 1.18 support #1116

merged 51 commits into from Aug 7, 2022

Conversation

flimzy
Copy link
Member

@flimzy flimzy commented Apr 19, 2022

This is based off of #1111, as we expect to merge that before the 1.18 work, and it made some of the changes easier, too.

Watch this space for updates.

nevkontakte and others added 23 commits May 30, 2022 19:11
All they do is insert fuzzing implementation stubs into the runtime
package, which our runtime doesn't use anyway. The stubs should be
discarded by dead code elimination automatically, so there is no
disadvantage compared to excluding the original source file.
Extracting and populating the list of fuzz targets will be done at a
later stage.
golang/go@1b2d794
in the upstream changed signatures of those functions in the reflect
package. The upstream `hiter` type is a close functional counterpart of
the `mapIter` type we already had, so I renamed `mapIter` into `hiter`
to match the upstream, and updated function implementations to match the
upstream signatures.
Corresponding upstream change:
golang/go@23832ba.

Our implementation is not, in fact, optimized, but at least it is
correct.
The JavaScript typed array that backs Go array must be shrunk to match
the expected Go type. If this is not done, copying the converted array
into an array of the same (expected) size causes a "source too large"
runtime exception.
The optimization this package implements relies heavily on finalizers,
which GopherJS doesn't support. As such it would create a memory leak
rather than an actual optimization. Until we decide that we can use
WeakMap API, this package can't be safely supported.

The only current use of this package is in the `net/netip` package and
this commit adds overlays that replace uses of the intern package with
direct use of strings.
In GopherJS this function is a no-op, since we don't have access to
low-level memory management. We also have to disable the corresponding
test, since it uses unsafe features we don't support.
Apparently, under V8 division by zero literal and by zero assigned to a
variable yields bitwise-different values of NaN, which broke some tests
in `internal/fuzz`. Using an actual NaN value from the JavaScript
runtime fixes the issue.

Isolated example: https://jsfiddle.net/j3rtaz0o/1/. Note that Firefox
doesn't seem to exhibit this issue.
This package uses generics in its tests, which we currently don't
support. This causes the compiler to crash.
Various standard library and compiler fixes to support Go 1.18
This function is used when an object method needs to be bound to the
receiver to be invoked elsewhere. Using bind() should be more efficient
here than wrapping it in an anonymous function, and it prevents
unexpected call frames from appearing in the stack trace.
This change makes the returned stack traces more similar to what you'd
get in a regular Go runtime. In particular, it skips certain
GopherJS-specific prelude functions like `$callDeferred()`. I expect we
will be adding to the list as we discover other edge cases.

We also rename certain functions to match their upstream counterparts,
since parts of the standard library rely on this. We mark them as
reserved symbols for prelude minification to make sure this doesn't
break when minified.
This prevents pc collision when minified and virtually all calls and
functions are on the same line.

This change resolves `testing.TestTBHelper` test failure in Go 1.18.
 - Avoid use of unsafe in TestNestedMethods and TestEmbeddedMethods.
 - Skip TestNotInHeapDeref and TestMethodCallValueCodePtr, which concern
   low-level GC and heap details, inapplicable to GopherJS.
 - Skip TestIssue50208 until generics are supported.
 - Add a hack into methodNameSkip() to make method names match what the
   tests expect. #1085 would
   be a better long-term solution.
 - Stub out verifyNotInHeapPtr(), which is also not applicable to
   GopherJS.
Fix remaining standard library test failures
@github-actions

This comment has been minimized.

nevkontakte and others added 10 commits July 21, 2022 22:51
This isn't a new regression in Go 1.18 and there seem to be several
distinct bugs affecting it, so it's reasonable to address it as a
separate issue: #1128.
Although we could support this function, we currently do not and the
`unsafe` package generally has limited support status.

Filed #1130 to track this.
Triage remaining gorepo test failures in 1.18
This should de-clutter tool.go a bit and help with maintainability in
future.
One of the helper functions requires generics to work correctly. Until
they are properly supported we have to stub it out.

Generics support is tracked in
#1013.
Detect and execute fuzz targets as tests
@github-actions

This comment has been minimized.

nevkontakte and others added 11 commits August 1, 2022 16:09
Even though the compiler was usually able to compile them into
_something_, the code was likely incorrect in all cases. To prevent
users from tripping up on that the compiler will return an error if it
encounters type params until
#1013 is resolved.
Go 1.18 upstream includes the fix, which makes the overlay unnecessary.
This mirrors the change in the upstream.
Upstream already skips it for wasm, so it should be ok for us to skip it
as well.
GopherJS doesn't have the problem it targets, but its println() format
is different from upstream Go, which causes the test failure.
@nevkontakte nevkontakte changed the title Starting on the 1.18 update Add Go 1.18 support Aug 1, 2022
@github-actions
Copy link

github-actions bot commented Aug 1, 2022

Reference app: jQuery TodoMVC (acf500a6c32a83d8c4582d967b09a65febf0e120)

BRANCH ORIGINAL MINIFIED COMPRESSED (GZIP)
Pull request (go1.18) 3,185,360 bytes 2,077,087 bytes 393,759 bytes
Target branch (master) 2.09% increase (65,119 bytes) 2.05% increase (41,640 bytes) 1.48% increase (5,757 bytes)

#outputSize

@nevkontakte
Copy link
Member

I think we should be good to merge this into master now? Although I would like to sneak in a couple of small ES 2015-related changes before we tag the release.

@nevkontakte nevkontakte marked this pull request as ready for review August 7, 2022 11:23
@nevkontakte
Copy link
Member

Since no objections have been made, I'm merging this branch to master 🎉 I intend to tag a 1.18beta1 release later this week.

@nevkontakte nevkontakte merged commit fa8544d into master Aug 7, 2022
@nevkontakte nevkontakte deleted the go1.18 branch August 7, 2022 11:24
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.

None yet

2 participants