compiler: conform to latest iteration of wasm types proposal #4501

ydnar · 2024-10-05T22:13:49Z

Updates to current version of the wasm types proposal at golang/go#66984.

Changes include:

Removal of int and uint as allowed types.
Restriction of int8, uint8, int16, and uint16 to struct fields, arrays, or pointer values.
Requires struct types in wasm params with 1 or more fields to include structs.HostLayout. It enforces this for Go 1.23 or later.
- TODO: revise this to minimum Go 1.24?

ydnar · 2024-10-07T22:56:22Z

@dgryski @aykevl would you prefer I split this into two parts, one that upgrades the wasm-tools-go dependency and regenerates the bindings, and a separate one with the wasm types changes to package compiler?

deadprogram · 2024-10-17T07:07:03Z

would you prefer I split this into two parts, one that upgrades the wasm-tools-go dependency and regenerates the bindings, and a separate one with the wasm types changes to package compiler?

I personally would prefer that. 😸

ydnar · 2024-10-17T15:18:09Z

would you prefer I split this into two parts, one that upgrades the wasm-tools-go dependency and regenerates the bindings, and a separate one with the wasm types changes to package compiler?

I personally would prefer that. 😸

Here you go: #4529

deadprogram · 2024-10-17T16:29:31Z

# github.com/tinygo-org/tinygo/compiler
compiler/symbol.go:469:33: c.pkg.GoVersion undefined (type *types.Package has no field or method GoVersion)

https://app.circleci.com/pipelines/github/tinygo-org/tinygo/11640/workflows/e46166fe-40b9-410a-95a9-d49be472b0d1/jobs/44676?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link&utm_content=summary&invite=true#step-112-0_39

deadprogram · 2024-10-17T19:49:35Z

@ydnar note the CI failure...

ydnar · 2024-10-17T19:50:15Z

What's the minimum version of Go supported by TinyGo?

Looks like this field was added in 1.21: https://pkg.go.dev/go/types#Package.GoVersion

deadprogram · 2024-10-17T19:56:07Z

"Officially" should be same as Go itself. But we have been able to keep backwards compat quite a few versions of both Go and LLVM, as shown by the CircleCI tests.

ydnar · 2024-10-17T21:09:57Z

"Officially" should be same as Go itself. But we have been able to keep backwards compat quite a few versions of both Go and LLVM, as shown by the CircleCI tests.

How does this project define compatibility?

TinyGo can be built with version N of Go?
or TinyGo can be used with version N of Go?

Asking because what % of users build TinyGo from source, and what % of those users are using older versions of Go? Versus what % of end-users of TinyGo who use a binary distribution use older versions of Go?

aykevl · 2024-10-18T08:22:05Z

Usually we keep compatibility as long as it isn't too much of an issue to support. We tend to drop older versions when it's getting annoying to support these older versions. It's always surprising how many people rely on older Go versions but still expect to use a newer TinyGo version.

Also, to be clear: because it makes CI easier "can be built" and "can be used with" is usually the same version. The lower bound is currently LLVM 15 and Go 1.19:

tinygo/.circleci/config.yml

Lines 121 to 123 in ef4f46f

    
           # This tests our lowest supported versions of Go and LLVM, to make sure at 
        
           # least the smoke tests still pass. 
        
           - test-llvm15-go119

aykevl

LGTM, though please see #4520 because it will conflict.

goenv/version.go

aykevl · 2024-10-18T08:29:52Z

compiler/symbol.go

+		if !goenv.WantGoVersion(c.pkg.GoVersion(), 1, 23) {
+			hasHostLayout = true // package structs does not exist before go1.23


I guess one way to work around this is to put this in a separate file with build tags. Together with #4501, the compiler won't try to compile projects with a Go version that's newer than the one TinyGo was compiled with (which means that when TinyGo was compiled with Go before 1.23, it will refuse to even compile code that requires Go 1.23+).

It's a bit ugly, but will keep compatibility for a bit longer.

I can do this.

@aykevl thanks! I rebased and my branch now uses Parse. I included the tests for Parse in my PR as well.

goenv/version.go

aykevl · 2024-10-18T08:39:27Z

I guess we could drop support for Go 1.19 and Go 1.20. That would leave Go 1.21-1.23 supported. I find it difficult to guess how many people would have an issue with this (note that Go 1.19 is the Go version in Debian 12).

Address PR feedback in tinygo-org#4501.

ydnar · 2024-10-18T18:42:37Z

@aykevl I updated this quite a bit. Can you take a look?

ydnar · 2024-10-18T21:32:22Z

I can split this into two PRs, one for the package goenv changes (Parse and Compare), and a second for the wasm types.

deadprogram · 2024-10-19T09:07:43Z

I can split this into two PRs, one for the package goenv changes (Parse and Compare), and a second for the wasm types.

Sorry to be a pain, but I am fairly certain that will make it a lot easier to review. Thank you!

Address PR feedback in tinygo-org#4501.

ydnar · 2024-10-19T20:04:46Z

I can split this into two PRs, one for the package goenv changes (Parse and Compare), and a second for the wasm types.

Sorry to be a pain, but I am fairly certain that will make it a lot easier to review. Thank you!

Done! See #4536.

deadprogram · 2024-10-21T14:13:09Z

@ydnar I think this PR can be rebased now, if you please.

golang/go#66984 - Remove int and uint as allowed types in params, results, pointers, or struct fields - Only allow small integers in pointers, arrays, or struct fields

golang/go#66984 TODO: regenerate WASI syscall packages for GC shape types including structs.HostLayout compiler: require go1.23 for structs.HostLayout compiler: use an interface to check if GoVersion() exists This permits TinyGo to compile with Go 1.21. compiler: use goenv.Compare instead of WantGoVersion

compiler/testdata: improve tests for structs.HostLayout

ydnar · 2024-10-21T14:41:24Z

@deadprogram done!

deadprogram

Did some testing and my current code still compiles and runs without issue.

LGTM!

aykevl · 2024-10-22T13:11:49Z

compiler/symbol.go

+		if gv, ok := any(c.pkg).(interface{ GoVersion() string }); ok {
+			if goenv.Compare(gv.GoVersion(), "go1.23") >= 0 {
+				hasHostLayout = false // package structs added in go1.23


Interesting solution, I hadn't thought of that! Much cleaner than using build tags.

Yeah! It's great when the only difference is a new method.

aykevl · 2024-10-22T13:14:43Z

Was this PR also meant to be squashed?

deadprogram · 2024-10-22T16:03:51Z

Now merging, thank you @ydnar for working on this and to @aykevl for helping review.

ydnar requested review from aykevl and dgryski October 5, 2024 22:13

ydnar self-assigned this Oct 5, 2024

ydnar changed the base branch from release to dev October 5, 2024 22:14

ydnar mentioned this pull request Oct 5, 2024

cmd/compile: relax wasm/wasm32 function import signature type constraints golang/go#66984

Closed

ydnar changed the title ~~compiler: update to latest version of wasm types proposal~~ compiler: conform to latest iteration of wasm types proposal Oct 5, 2024

ydnar mentioned this pull request Oct 17, 2024

internal: update wasm-tools-go to v0.3.0 #4529

Merged

ydnar force-pushed the ydnar/wasm-types branch from 9972fcc to 94e3bf9 Compare October 17, 2024 15:21

ydnar force-pushed the ydnar/wasm-types branch from 94e3bf9 to 6917156 Compare October 17, 2024 21:06

aykevl reviewed Oct 18, 2024

View reviewed changes

ydnar force-pushed the ydnar/wasm-types branch from 6917156 to e945a4f Compare October 18, 2024 16:33

ydnar added a commit to ydnar/tinygo that referenced this pull request Oct 18, 2024

goenv: improve WantGoVersion

35d8224

Address PR feedback in tinygo-org#4501.

ydnar requested a review from aykevl October 18, 2024 18:43

ydnar added a commit to ydnar/tinygo that referenced this pull request Oct 19, 2024

goenv: improve WantGoVersion

53a9418

Address PR feedback in tinygo-org#4501.

ydnar force-pushed the ydnar/wasm-types branch from 7f96e9f to ae5801f Compare October 19, 2024 19:57

ydnar added a commit to ydnar/tinygo that referenced this pull request Oct 19, 2024

goenv: improve WantGoVersion

10b3aad

Address PR feedback in tinygo-org#4501.

ydnar force-pushed the ydnar/wasm-types branch from ae5801f to bb5805a Compare October 19, 2024 20:00

ydnar mentioned this pull request Oct 19, 2024

goenv: parse patch version, add func Compare to compare two Go version strings #4536

Merged

ydnar added 4 commits October 21, 2024 07:37

compiler: align with current wasm types proposal

7dabd72

golang/go#66984 - Remove int and uint as allowed types in params, results, pointers, or struct fields - Only allow small integers in pointers, arrays, or struct fields

testdata/wasmexport: use int32 instead of int

4ebecef

compiler/testdata: add structs.HostLayout

9d2bf5f

compiler/testdata: improve tests for structs.HostLayout

ydnar force-pushed the ydnar/wasm-types branch from bb5805a to 9d2bf5f Compare October 21, 2024 14:38

deadprogram approved these changes Oct 22, 2024

View reviewed changes

aykevl approved these changes Oct 22, 2024

View reviewed changes

deadprogram merged commit 24c11d4 into tinygo-org:dev Oct 22, 2024
17 checks passed

ydnar deleted the ydnar/wasm-types branch October 22, 2024 22:30

		if !goenv.WantGoVersion(c.pkg.GoVersion(), 1, 23) {
		hasHostLayout = true // package structs does not exist before go1.23

compiler: conform to latest iteration of wasm types proposal #4501

compiler: conform to latest iteration of wasm types proposal #4501

Uh oh!

Conversation

ydnar commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ydnar commented Oct 7, 2024

Uh oh!

deadprogram commented Oct 17, 2024

Uh oh!

ydnar commented Oct 17, 2024

Uh oh!

deadprogram commented Oct 17, 2024

Uh oh!

deadprogram commented Oct 17, 2024

Uh oh!

ydnar commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deadprogram commented Oct 17, 2024

Uh oh!

ydnar commented Oct 17, 2024

Uh oh!

aykevl commented Oct 18, 2024

Uh oh!

aykevl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aykevl Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

ydnar Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

ydnar Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aykevl commented Oct 18, 2024

Uh oh!

ydnar commented Oct 18, 2024

Uh oh!

ydnar commented Oct 18, 2024

Uh oh!

deadprogram commented Oct 19, 2024

Uh oh!

ydnar commented Oct 19, 2024

Uh oh!

deadprogram commented Oct 21, 2024

Uh oh!

ydnar commented Oct 21, 2024

Uh oh!

deadprogram left a comment

Choose a reason for hiding this comment

Uh oh!

aykevl Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

ydnar Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

aykevl commented Oct 22, 2024

Uh oh!

deadprogram commented Oct 22, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ydnar commented Oct 5, 2024 •

edited

Loading

ydnar commented Oct 17, 2024 •

edited

Loading