-
Notifications
You must be signed in to change notification settings - Fork 570
Add Go 1.18 support #1116
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
Add Go 1.18 support #1116
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
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
This comment has been minimized.
This comment has been minimized.
This addresses golang/go#49110 for GopherJS.
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
This comment has been minimized.
This comment has been minimized.
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.
Final cleanups for Go 1.18
Reference app: jQuery TodoMVC (
#outputSize |
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
approved these changes
Aug 7, 2022
Since no objections have been made, I'm merging this branch to master 🎉 I intend to tag a 1.18beta1 release later this week. |
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.
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.