-
Notifications
You must be signed in to change notification settings - Fork 570
Go 1.10 support (version bump to GopherJS 1.10-1). #755
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
Conversation
…ncname. These are used by the new panicwrap implementation. panicwrap wasn't implemented previously either, so this is not a regression. Related to golang/go@354fa9a. Fixes: runtime/error.go:134:8: undeclared name: getcallerpc runtime/error.go:135:10: undeclared name: funcname runtime/error.go:135:19: undeclared name: findfunc
We can now use the upstream one from package math. In Go 1.10, it's now implemented directly. Previously, it relied on max and had assembly implementations, which is why a custom implementation for GopherJS was needed. Related to golang/go@b97688d. Fixes: math/math.go:67:9: undeclared name: dim
In Go 1.10, package time has a new LoadLocationFromTZData API addition. There were some internal code changes, and this change makes appropriate updates to GopherJS. Fixes: /src/time/time.go:83:9: undeclared name: loadZoneFile /usr/local/go/src/time/zoneinfo.go:302:28: undeclared name: zoneSources
It always had the zero value and was unused. It was removed upstream. Keep name.pkgPath method in order to override the upstream method that remains with non-trivial implementation (one that would fail). See https://github.com/golang/go/blob/bfb8f2a765d6097c82b30ae2c19c99fa1082add3/src/reflect/type.go#L515-L529 and https://go-review.googlesource.com/c/go/+/54331/3/src/reflect/type.go#b515. Related to golang/go@9c9df65 (/cc @mvdan). Fixes: reflect/type.go:1451:45: cannot convert false (untyped bool constant) to string reflect/type.go:1451:50: too few arguments in call to newName reflect/type.go:1854:45: cannot convert false (untyped bool constant) to string reflect/type.go:1854:50: too few arguments in call to newName reflect/type.go:1897:45: cannot convert false (untyped bool constant) to string reflect/type.go:1897:50: too few arguments in call to newName reflect/type.go:2065:47: cannot convert false (untyped bool constant) to string reflect/type.go:2065:52: too few arguments in call to newName reflect/type.go:2260:44: cannot convert false (untyped bool constant) to string reflect/type.go:2260:49: too few arguments in call to newName reflect/type.go:2260:49: too many errors
…m uint64 to int. It has changed upstream. Related to golang/go@8a6e51a. Fixes: reflect/value.go:2119:31: cannot use buffer (variable of type int) as uint64 value in argument to makechan
syscall.Exit implementation has been removed upstream. In Go 1.10, it's provided via runtime.exit. We can't do that, so just implement syscall.Exit ourselves as it was previously implemented. Related to golang/go@438c8f6. Fixes: Error: runtime error: native function not implemented: syscall.Exit
Copy semaphore implementation from sync package. In Go 1.10, these are used in FD.destroy and FD.Close methods. Related to golang/go@382d492. Fixes: Error: runtime error: native function not implemented: internal/poll.runtime_Semrelease
The behavior or reflect.Copy has changed upstream to somewhat mirror the special case behavior of the copy built-in: // As a special case, src can have kind String // if the element type of dst is kind Uint8. Related to golang/go@245e386. Fixes: Error: reflect: call of reflect.Copy on string Value
…ded fields. Update for upstream changes. Related to golang/go@6471ace. Related to golang/go@d349fa2. Fixes: === RUN TestCallPanic --- FAIL: TestCallPanic (0.01s) /Users/Dmitri/Dropbox/Work/2013/GoLand/src/github.com/gopherjs/gopherjs/ test.043844921:1412 throw new Error(msg); ^ Error: did not panic
…ported methods. Update overridden method to keep up to date with an upstream change. Related to golang/go@6f1724f.
At least this helps narrow down where the issue was. Need to come up with a general, correct fix. Related to golang/go@6f1724f.
…ypes. I don't see a good reason to skip string internalization in reflect package. It was introduced as part of other changes for Go 1.7 support in commit f1514b4, but no rationale was provided. Skipping internalization is inconsistent and causes issues because string comparison no longer works as expected. Specifically, the test TestMethodByNameUnExportedFirst was failing with: got , expected ΦExported The reason it was failing is because the test makes a typ.MethodByName("ΦExported") call. Once that string is internalized, it becomes "\xCE\xA6Exported". As a result, the code in rtype.MethodByName was failing to find the method by name, because the internalized and non-internalized strings did not match: have: unexported want: ΦExported match: false have: ΦExported want: ΦExported match: false Removing this special behavior seems to not have any adverse effects, fixes the issue and makes things simpler: have: unexported want: ΦExported match: false have: ΦExported want: ΦExported match: true
It uses runtime.SetFinalizer, which is not supported by GopherJS.
…verted nil interface. Not yet sure if this is the right/best fix, needs investigation. But it allows the TestCallConvert test to pass without breaking any others. Related to golang/go@eafa29b. Fixes: $ gopherjs test -v --run=TestCallConvert reflect === RUN TestCallConvert --- FAIL: TestCallConvert (0.10s) all_test.go:1639: expected [nil], got [<io.Reader Value>] FAIL FAIL reflect 0.808s
…lers. frameSkip relies on runtime.Callers when testing.T.Helper was called. Fixes: $ gopherjs test --run=TestBuilder strings --- FAIL: TestBuilder (0.07s) github.com/gopherjs/gopherjs/test.481734121:1412 throw new Error(msg); ^ Error: testing: zero callers found
…chitecture. Can't use package unsafe, so use a simple type conversion from []byte to string in Builder.String method. Further optimizations are deferred for later (most likely not needed because Go->wasm will be ready by the time they're needed). Related to golang/go#18990.
It's being used in a few places, and previously had no implementation in GopherJS. (In upstream Go, it's being provided by runtime via go:linkname directive.) This restores more helpful error messages as in GopherJS 1.9. Before: runtime error: native function not implemented: sync.throw After: runtime error: sync: inconsistent mutex state
…on for types." This reverts commit 2f2d5b3. That commit (and its commit message) was wrong/incomplete. Now I understand there was a reason for skipping string internalization for types. The following commit is a better alternative fix.
It has been removed in go1.10beta2.
…gesize. Fixes: --- FAIL: TestIndexByteNearPageBoundary (0.00s) Error: runtime error: native function not implemented: syscall.Getpagesize FAIL bytes 19.341s
Since the package files are excluded too, the tests can't compile. Fixes: ../../../usr/local/go/src/crypto/rand/rand_linux_test.go:13:17: undeclared name: batched ../../../usr/local/go/src/crypto/rand/rand_linux_test.go:31:7: undeclared name: batched ../../../usr/local/go/src/crypto/rand/rand_linux_test.go:38:7: undeclared name: batched
This helps know which parts of the package can be relied on, or not.
It's being done upstream to trick the escape analysis and prevent allocations. We can't rely on it here. Fixes: === RUN TestBuilder --- FAIL: TestBuilder (0.01s) TypeError: Cannot create property '$proxies' on number '0'
TestMultiWriterSingleChainFlatten was added in Go 1.10, and it needs to be skipped (just like TestMultiReaderFlatten) because it relies on unsupported runtime features. Related to golang/go@060d1a5. Fixes: === RUN TestMultiWriterSingleChainFlatten --- FAIL: TestMultiWriterSingleChainFlatten (0.01s) test.558082205:38: multiWriter did not flatten chained multiWriters: expected writeDepth 12, got 4 FAIL io 0.888s
TestTypeRace was added in Go 1.10, and it needs to be skipped because it relies on sync.WaitGroup to behave correctly under concurrency. However, encoding/gob is currently configured to use nosync, which cannot succeed. Since this is a new test, opt to skip it for now rather than making it succeed by replacing nosync with sync. Related to golang/go@1a9f27d. Fixes: === RUN TestTypeRace --- FAIL: TestTypeRace (0.00s) Error: sync: WaitGroup counter not zero FAIL encoding/gob 3.030s
…itly converted nil interface." This reverts commit 1fb1aaa. The change in that commit was not a complete and correct solution. Instead of resolving the underlying problem, it masked a symptom, allowing the TestCallConvert test to pass. A better fix will follow next.
…Kind() == Interface && v.IsNil(). This change adds a copy of Value.assignTo method to reflect package, identical to upstream, except with golang/go@eafa29b not applied. That commit adds a short-circuit check to that skips ifaceE2I, in order to avoid a panic that occurs in ifaceE2I under that condition in gc and gccgo compilers. GopherJS implements a custom version of ifaceE2I which needs to run and does not panic in that condition. So, there's no need skip it. Fixes: $ gopherjs test --short reflect html/template --- FAIL: TestCallConvert (0.01s) all_test.go:1639: expected [nil], got [<io.Reader Value>] FAIL FAIL reflect 3.084s --- FAIL: TestEscapingNilNonemptyInterfaces (0.01s) go/src/github.com/shurcooL/play/241/test.243863553:9608 if (!(($parseInt(methodSet.length) === 0)) || !!(typ.named)) { ^ TypeError: Cannot read property 'length' of undefined at reflectType (go/src/github.com/shurcooL/play/241/test.243863553:9608:31) at Object.TypeOf (go/src/github.com/shurcooL/play/241/reflect.go:312:3) at indirect (/html/template/content.go:119:6) at stringify (/html/template/content.go:153:4) at htmlEscaper (/html/template/html.go:43:3) at $call (go/src/github.com/shurcooL/play/241/test.243863553:30:50) at go/src/github.com/shurcooL/play/241/test.243863553:1948:22 at Object.$packages.reflect.Value.ptr.call (go/src/github.com/shurcooL/play/241/reflect.go:880:3) at Object.$packages.reflect.Value.ptr.Call (/reflect/value.go:308:3) at Object.$packages.text/template.state.ptr.evalCall (/text/template/exec.go:667:3) at Object.$packages.text/template.state.ptr.evalFunction (/text/template/exec.go:535:3) at Object.$packages.text/template.state.ptr.evalCommand (/text/template/exec.go:432:4) at Object.$packages.text/template.state.ptr.evalPipeline (/text/template/exec.go:405:4) at Object.$packages.text/template.state.ptr.walk (/text/template/exec.go:231:4) at Object.$packages.text/template.state.ptr.walk (/text/template/exec.go:239:5) at Object.$packages.text/template.Template.ptr.execute (/text/template/exec.go:194:3) at Object.$packages.text/template.Template.ptr.Execute (/text/template/exec.go:177:3) at Object.$packages.html/template.Template.ptr.Execute (/html/template/template.go:122:3) at Object.TestEscapingNilNonemptyInterfaces [as $blk] (/html/template/content_test.go:448:3) at Object.tRunner [as $blk] (/testing/testing.go:777:3) at fun (go/src/github.com/shurcooL/play/241/test.243863553:1479:37) at $goroutine (go/src/github.com/shurcooL/play/241/test.243863553:1477:19) at $runScheduled (go/src/github.com/shurcooL/play/241/test.243863553:1517:7) at $schedule (go/src/github.com/shurcooL/play/241/test.243863553:1533:5) at $go (go/src/github.com/shurcooL/play/241/test.243863553:1509:3) at Object.<anonymous> (go/src/github.com/shurcooL/play/241/test.243863553:55363:1) at Object.<anonymous> (go/src/github.com/shurcooL/play/241/test.243863553:55366:4) at Module._compile (module.js:660:30) at Object.Module._extensions..js (module.js:671:10) at Module.load (module.js:573:32) at tryModuleLoad (module.js:513:12) at Function.Module._load (module.js:505:3) at Function.Module.runMain (module.js:701:10) at startup (bootstrap_node.js:190:16) at bootstrap_node.js:662:3 FAIL html/template 1.079s
Amazing work, as usual @shurcooL. I'll try this out on my projects today and let you know if I run into any problems. |
All good here, excited to have this merged when you're happy to do so. |
go generate ./...
This PR has been open for a week, and there've been no code review comments, nor issues reported. I'm reverting a temporary hacky fix in commit c4db7b2 that was applied for debugging purposes (reverted in commit 88dc1af), and opening #763 to track progress on resolving that issue. It's not a blocker for release: it only affects reflect code that touches methods sets that include exported methods starting with non-ASCII letters, which is rare. Tomorrow (Monday), I'm going to bump the version from "GopherJS 1.10-wip" to "GopherJS 1.10-1" and merge this. There'll be an announcement blog post and a tweet. The GopherJS Playground will be updated at the same time. |
@shurcooL - apologies, not had much time recently, hence not had a chance to look at the detail of this PR. For what it's worth I've just tried this out against my React work (links just above this comment) and all looks ok, both for regular files and minified output. |
…d64). Regenerate with: go generate github.com/gopherjs/gopherjs.github.io/playground Follows gopherjs/gopherjs#755.
GopherJS 1.10-1 is now released and available on Blog post: https://medium.com/gopherjs/gopherjs-1-10-1-is-released-2ff9002a6712. Tweet: https://twitter.com/GopherJS/status/968207925477965824. GopherJS Playground has been updated: https://gopherjs.github.io/playground/#/UdyEyAtguS. Thanks everyone! 🎉 🏄♂️ |
This PR adds support for Go 1.10, which has been released today.
It is passing CI ✅ on Linux (see here), and all tests are passing on macOS for me locally (aside from an unrelated upstream issue golang/go#23881). It's fully ready for testing and review!
(I have not tested it on Windows and can't guarantee support there.)
I've been testing the
go1.10
branch on all of my GopherJS packages during the last few weeks, and there are no known show-stoppers at this time.The aspirational plan is to merge this sometime next week, after it has a chance to go through review and additional testing, unless major show-stopping issues are discovered and take longer to fix. Minor non-critical issues don't need to be resolved as part of this PR, they can be worked on post-release. But any viable opportunities to improve code can be applied.
Please see issue #725 for context. Each individual commit has a commit message that describes what it's doing.
Resolves #725.