Support Go 1.14.

[QuLogic]

I believe this works without issues, but I've also got to figure out LLVM 10 at the same time. I also have only checked 0.11.0, not 0.12.0 yet. So let's see how CI feels about this.

[QuLogic]

Ah, took a few tries, but finally some actual errors. I guess I didn't see them yet since I usually run smoke test first, which hit LLVM 10 issues.

[QuLogic]

The failure in TestCGo/types appears to just be some line breaks:

diff --git a/cgo/testdata/types.out.go b/cgo/testdata/types.out.go
index 8a75008..1864e33 100644
--- a/cgo/testdata/types.out.go
+++ b/cgo/testdata/types.out.go
@@ -62,12 +62,16 @@ type C.union3_t = C.union_1
 type C.union_nested_t = C.union_3
 type C.unionarray_t = struct{ arr [10]C.uchar }
 
-func (s *C.struct_4) bitfield_a() C.uchar          { return s.__bitfield_1 & 0x1f }
-func (s *C.struct_4) set_bitfield_a(value C.uchar) { s.__bitfield_1 = s.__bitfield_1&^0x1f | value&0x1f<<0 }
+func (s *C.struct_4) bitfield_a() C.uchar { return s.__bitfield_1 & 0x1f }
+func (s *C.struct_4) set_bitfield_a(value C.uchar) {
+	s.__bitfield_1 = s.__bitfield_1&^0x1f | value&0x1f<<0
+}
 func (s *C.struct_4) bitfield_b() C.uchar {
 	return s.__bitfield_1 >> 5 & 0x1
 }
-func (s *C.struct_4) set_bitfield_b(value C.uchar) { s.__bitfield_1 = s.__bitfield_1&^0x20 | value&0x1<<5 }
+func (s *C.struct_4) set_bitfield_b(value C.uchar) {
+	s.__bitfield_1 = s.__bitfield_1&^0x20 | value&0x1<<5
+}
 func (s *C.struct_4) bitfield_c() C.uchar {
 	return s.__bitfield_1 >> 6
 }
@@ -100,19 +104,31 @@ func (union *C.union_1) unionfield_s() *C.short { return (*C.short)(unsafe.Point
 
 type C.union_1 struct{ $union uint64 }
 
-func (union *C.union_2) unionfield_area() *C.point2d_t  { return (*C.point2d_t)(unsafe.Pointer(&union.$union)) }
-func (union *C.union_2) unionfield_solid() *C.point3d_t { return (*C.point3d_t)(unsafe.Pointer(&union.$union)) }
+func (union *C.union_2) unionfield_area() *C.point2d_t {
+	return (*C.point2d_t)(unsafe.Pointer(&union.$union))
+}
+func (union *C.union_2) unionfield_solid() *C.point3d_t {
+	return (*C.point3d_t)(unsafe.Pointer(&union.$union))
+}
 
 type C.union_2 struct{ $union [3]uint32 }
 
-func (union *C.union_3) unionfield_point() *C.point3d_t    { return (*C.point3d_t)(unsafe.Pointer(&union.$union)) }
-func (union *C.union_3) unionfield_array() *C.unionarray_t { return (*C.unionarray_t)(unsafe.Pointer(&union.$union)) }
-func (union *C.union_3) unionfield_thing() *C.union3_t     { return (*C.union3_t)(unsafe.Pointer(&union.$union)) }
+func (union *C.union_3) unionfield_point() *C.point3d_t {
+	return (*C.point3d_t)(unsafe.Pointer(&union.$union))
+}
+func (union *C.union_3) unionfield_array() *C.unionarray_t {
+	return (*C.unionarray_t)(unsafe.Pointer(&union.$union))
+}
+func (union *C.union_3) unionfield_thing() *C.union3_t {
+	return (*C.union3_t)(unsafe.Pointer(&union.$union))
+}
 
 type C.union_3 struct{ $union [2]uint64 }
 
-func (union *C.union_union2d) unionfield_i() *C.int      { return (*C.int)(unsafe.Pointer(&union.$union)) }
-func (union *C.union_union2d) unionfield_d() *[2]float64 { return (*[2]float64)(unsafe.Pointer(&union.$union)) }
+func (union *C.union_union2d) unionfield_i() *C.int { return (*C.int)(unsafe.Pointer(&union.$union)) }
+func (union *C.union_union2d) unionfield_d() *[2]float64 {
+	return (*[2]float64)(unsafe.Pointer(&union.$union))
+}
 
 type C.union_union2d struct{ $union [2]uint64 }
 type C.enum_option C.int

The others look to be the same "branch on a non-constant":

• TestCompiler/EmulatedCortexM3/stdlib.go: main_test.go:139: failed to build: interp: branch on a non-constant
• TestCompiler/Host/stdlib.go: main_test.go:139: failed to build: interp: branch on a non-constant
• TestCompiler/WebAssembly/stdlib.go: main_test.go:139: failed to build: interp: branch on a non-constant
• TestCompiler/ARM64Linux/stdlib.go: main_test.go:139: failed to build: interp: branch on a non-constant

plus some odd things about node, which might be related to the above.

[aykevl]

I really don't like the idea of having different tests for different versions of Go. Especially when the differences are basically just gofmt changes. I think this could be solved with a regex to make sure the output matches the Go 1.14 formatting (so that in a few years from now, we could simply drop the regex when Go 1.13 and below are no longer supported).

Regarding the other errors, I suspect they are variations on #437

[QuLogic]

I suppose we could replace all newlines with a space, and then replace all space runs with a single space. Though that's broken code, I guess it would still check what's really important. Otherwise I'm not sure there's an easy regex to fix it as I don't know what triggers the extra wrapping.

[QuLogic]

That's not to say I can't write a regex for it; it'll just seem a bit arbitrary.

[aykevl]

My idea was more like this, to recognize the following pattern using a regex:

func <stuff>    { <stuff> }

and replace it with this:

func <stuff> {
    <stuff>
}

[QuLogic]

OK, that's also doable, but it doesn't always affect every func, unless you just want to do it for all of them?

[QuLogic]

I pushed a change that does the wrapping conservatively, only on the lines that changed. If you would prefer the wrapping to be always applied, that could be done instead.

[deadprogram]

We are going to need this sooner rather than later, especially now that Homebrew installs Go 1.14 by default.

[deadprogram]

Hopefully some work can be done on this 🙏

[QuLogic]

Rebased so we can get nice things like 982b2d0 in the output.

[QuLogic]

So it's pointing at math/rand (at least):

    TestCompiler/ARMLinux/stdlib.go: main.go:663: # math/rand
    TestCompiler/ARMLinux/stdlib.go: main.go:665: interp: branch on a non-constant

The diff between 1.13.8 and 1.14 for that directory is not very large: golang/go@go1.13.8...go1.14#diff-6e6d9989336ec13fdae43d4534e5f162

--- /usr/lib/golang1.13.8/src/math/rand/rand.go	2020-01-09 14:05:25.000000000 -0500
+++ /usr/lib/golang1.14/src/math/rand/rand.go	2020-02-25 13:32:55.000000000 -0500
@@ -261,15 +261,20 @@
 	if lk, ok := r.src.(*lockedSource); ok {
 		return lk.read(p, &r.readVal, &r.readPos)
 	}
-	return read(p, r.Int63, &r.readVal, &r.readPos)
+	return read(p, r.src, &r.readVal, &r.readPos)
 }
 
-func read(p []byte, int63 func() int64, readVal *int64, readPos *int8) (n int, err error) {
+func read(p []byte, src Source, readVal *int64, readPos *int8) (n int, err error) {
 	pos := *readPos
 	val := *readVal
+	rng, _ := src.(*rngSource)
 	for n = 0; n < len(p); n++ {
 		if pos == 0 {
-			val = int63()
+			if rng != nil {
+				val = rng.Int63()
+			} else {
+				val = src.Int63()
+			}
 			pos = 7
 		}
 		p[n] = byte(val)
@@ -285,12 +290,15 @@
  * Top-level convenience functions
  */
 
-var globalRand = New(&lockedSource{src: NewSource(1).(Source64)})
+var globalRand = New(&lockedSource{src: NewSource(1).(*rngSource)})
+
+// Type assert that globalRand's source is a lockedSource whose src is a *rngSource.
+var _ *rngSource = globalRand.src.(*lockedSource).src
 
 // Seed uses the provided seed value to initialize the default Source to a
 // deterministic state. If Seed is not called, the generator behaves as
 // if seeded by Seed(1). Seed values that have the same remainder when
-// divided by 2^31-1 generate the same pseudo-random sequence.
+// divided by 2³¹-1 generate the same pseudo-random sequence.
 // Seed, unlike the Rand.Seed method, is safe for concurrent use.
 func Seed(seed int64) { globalRand.Seed(seed) }
 
@@ -373,7 +381,7 @@
 
 type lockedSource struct {
 	lk  sync.Mutex
-	src Source64
+	src *rngSource
 }
 
 func (r *lockedSource) Int63() (n int64) {
@@ -407,7 +415,7 @@
 // read implements Read for a lockedSource without a race condition.
 func (r *lockedSource) read(p []byte, readVal *int64, readPos *int8) (n int, err error) {
 	r.lk.Lock()
-	n, err = read(p, r.src.Int63, readVal, readPos)
+	n, err = read(p, r.src, readVal, readPos)
 	r.lk.Unlock()
 	return
 }

Is it the global type assert on rngSource that's bugging it? Unfortunately, the line number is totally bogus.

[aykevl]

I think it's this:

var _ *rngSource = globalRand.src.(*lockedSource).src

I'm working on a fix.

[aykevl]

Here is a partial fix: #967

[QuLogic]

Toggled the Debug flag in the tests, which gets us proper line numbers:

    TestCompiler/Host/stdlib.go: main.go:663: # math/rand
    TestCompiler/Host/stdlib.go: main.go:665: 296:31: interp: branch on a non-constant

and line 296 is indeed the rngSource type assert line.

[deadprogram]

@QuLogic can you please rebase against dev, since #967 has been merged? Thank you.

[QuLogic]

Unfortunately, I already did.

[aykevl]

This should fix the problem with math/rand: #983

[QuLogic]

Indeed, that fixes it and is a good catch; I was looking much deeper down the IR passes. Looks like the only thing left is some WASM stuff now.

[QuLogic]

OK, so the wasm failure is because wasm_exec.js is out of sync with upstream Go. The change the tests are tripping on is this addition of garbage collection. It seems somewhat straightforward to incorporate that, but there are many other changes to wasm_exec.js that maybe could be synced in too if you want to.

The main problem is going to be that the type flag numbers changed. The GC itself would probably be fine since older Go would just no unref things just like before.

[deadprogram]

I have been wanting to update the wasm_exec.js for a while now, we've gotten pretty far out of sync.

[aykevl]

The main problem is going to be that the type flag numbers changed.

Ouch, I'm afraid this may be a big problem. In the worst case we'll have to drop support for WebAssembly with Go 1.13 and lower.

[aykevl]

@QuLogic since #966 it is possible to select the Go version in Azure Pipelines. It is still at Go 1.13 with this PR. The macOS builds also have a fixed Go version in CI (CircleCI).
I think it would be a good idea to update this PR to switch both the Windows and the macOS builds over to Go 1.14 to have a better idea of what still needs to be done.

[aykevl]

I pushed a change that does the wrapping conservatively, only on the lines that changed. If you would prefer the wrapping to be always applied, that could be done instead.

Oops I only read this properly now.
Yes I would prefer the wrapping to be applied in all cases, to make the normalizing code simpler. In fact, I've already written that (dev...normalize-cgo-tests) and am now re-reading your comment again. So, feel free to take it (or not) for this PR.

[deadprogram]

Regarding dropping support for lower versions of wasm, that is not a great option for all of the various projects now using tinygo, but I suspect most of them would accept this tradeoff in return for the updated functionality.

Any comments from WebAssembly devs using TinyGo?

One thing we should try to consider on this round of updates, is how we can keep the wasm_exec.js file in better sync, or at least more frequent sync.

[QuLogic]

I've gone through most of the wasm_exec.js changes, but haven't posted them. I'll try and get that in a PR tomorrow.

[QuLogic]

I made PRs for the simple bits of syncing wasm_exec. What remains is the following:

• Changes to fix argv/envp layout; TinyGo's wasm_exec has removed support for these, and I don't know why that decision was made other than 'not working'. These fixes may make it work again.
• synchronous callbacks to js/wasm: golang/go@6dd70fc + various followup commits. This has lots of corresponding changes to src/runtime, so I think not compatible with Go 1.13.
• reuse wasm memory DataView: golang/go@07f0460. The new resetMemoryDataView needs to be called from the Go side.
• faketime renames: golang/go@4af3c17. It doesn't appear that TinyGo has any of the go section, so nanotime and walltime don't exist to be renamed.
• garbage collect references to JavaScript values: golang/go@54e6ba6. As mentioned earlier, the type flag values changed.

[deadprogram]

@QuLogic @aykevl @jaddr2line is that list of items the last outstanding items? And is the main decision to drop support for WASM on versions of Go less than 1.14? Just trying to understand what we need to do here.

[deadprogram]

Now that PR #1035 has been merged, I think this PR is no longer needed, and can be closed @QuLogic

[QuLogic]

Sorry, I got a bit too busy to get back to this; thanks for finishing it up @aykevl.