Merge pull request #286 from bytecodealliance/ydnar/issue284

wit/bindgen: do not use bool for result or variant shapes

Author: ydnar

CHANGELOG.md

@@ -14,6 +14,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 - Breaking: generated `*.wasm.go` files will now have correct WIT kebab-case base name. Interfaces or worlds with `-` in their name will require removal of the previous `*.wasm.go` files.
 - Dropped support for TinyGo v0.32.0.
 
+### Fixed
+
+- [#284](https://github.com/bytecodealliance/go-modules/issues/284): do not use `bool` for `variant` or `result` GC shapes. TinyGo returns `result` and `variant` values with `bool` as 0 or 1, which breaks the memory representation of tagged unions (variants).
+
 ## [v0.5.0] — 2024-12-14
 
 ### Changed

Makefile

@@ -53,3 +53,7 @@ test:
 	tinygo test -target=wasip1 $(GOTESTARGS) $(GOTESTMODULES)
 	tinygo test -target=wasip2 $(GOTESTARGS) $(GOTESTMODULES)
 	tinygo test -target=wasip2 $(GOTESTARGS) ./tests/...
+
+.PHONY: tests
+tests:
+	tinygo test -target=wasip2 $(GOTESTARGS) ./tests/...

cm/result_test.go

@@ -1,6 +1,7 @@
 package cm
 
 import (
+	"fmt"
 	"runtime"
 	"testing"
 	"unsafe"
@@ -238,3 +239,98 @@ func TestIssue95BoolInt64(t *testing.T) {
 		t.Errorf("*res.OK(): %v, expected %v", got, want)
 	}
 }
+
+func TestIssue284(t *testing.T) {
+	// Using int8 instead of bool for shape works on Go and TinyGo.
+	// See https://github.com/bytecodealliance/go-modules/issues/284
+	type BoolS8Result Result[int8, bool, int8]
+
+	tests := []struct {
+		isErr bool
+		ok    bool
+		err   int8
+	}{
+		{false, false, 0},
+		{false, true, 0},
+		{true, false, 0},
+		{true, false, 1},
+		{true, false, 5},
+		{true, false, 126},
+		{true, false, 127},
+	}
+	for _, tt := range tests {
+		if !tt.isErr {
+			t.Run(fmt.Sprintf("OK[BoolS8Result](%t)", tt.ok), func(t *testing.T) {
+				r := OK[BoolS8Result](tt.ok)
+				ok, _, isErr := r.Result()
+				if isErr != tt.isErr {
+					t.Errorf("isErr == %t, expected %t", isErr, tt.isErr)
+				}
+				if ok != tt.ok {
+					t.Errorf("ok == %t, expected %t", ok, tt.ok)
+				}
+			})
+		} else {
+			t.Run(fmt.Sprintf("Err[BoolS8Result](%d)", tt.err), func(t *testing.T) {
+				r := Err[BoolS8Result](tt.err)
+				_, err, isErr := r.Result()
+				if isErr != tt.isErr {
+					t.Errorf("isErr == %t, expected %t", isErr, tt.isErr)
+				}
+				if err != tt.err {
+					t.Errorf("err == %d, expected %d", err, tt.err)
+				}
+			})
+		}
+	}
+}
+
+func TestIssue284NotTinyGo(t *testing.T) {
+	if runtime.Compiler == "tinygo" {
+		return
+	}
+
+	// Using bool as result shape changes how [OK] returns the result by value.
+	// This works on Go, but breaks on TinyGo / LLVM.
+	// See https://github.com/bytecodealliance/go-modules/issues/284
+	type BoolS8Result Result[bool, bool, int8]
+
+	tests := []struct {
+		isErr bool
+		ok    bool
+		err   int8
+	}{
+		{false, false, 0},
+		{false, true, 0},
+		{true, false, 0},
+		{true, false, 1},
+		{true, false, 5},
+		{true, false, 126},
+		{true, false, 127},
+	}
+	for _, tt := range tests {
+		if !tt.isErr {
+			t.Run(fmt.Sprintf("OK[BoolS8Result](%t)", tt.ok), func(t *testing.T) {
+				r := OK[BoolS8Result](tt.ok)
+				ok, _, isErr := r.Result()
+				if isErr != tt.isErr {
+					t.Errorf("isErr == %t, expected %t", isErr, tt.isErr)
+				}
+				if ok != tt.ok {
+					t.Errorf("ok == %t, expected %t", ok, tt.ok)
+				}
+			})
+		} else {
+			t.Run(fmt.Sprintf("Err[BoolS8Result](%d)", tt.err), func(t *testing.T) {
+				r := Err[BoolS8Result](tt.err)
+				_, err, isErr := r.Result()
+				if isErr != tt.isErr {
+					t.Errorf("isErr == %t, expected %t", isErr, tt.isErr)
+				}
+				if err != tt.err {
+					t.Errorf("err == %d, expected %d", err, tt.err)
+				}
+			})
+		}
+	}
+}

tests/generated/issues/issue284/i/i.wit.go

@@ -6,7 +6,7 @@ import (
 	"go.bytecodealliance.org/wit"
 )
 
-// variantShape returns the type with the greatest size.
+// variantShape returns the type with the greatest size that is not a bool.
 // If there are multiple types with the same size, it returns
 // the first type that contains a pointer.
 func variantShape(types []wit.Type) wit.Type {
@@ -19,6 +19,12 @@ func variantShape(types []wit.Type) wit.Type {
 			return -1
 		case a.Size() < b.Size():
 			return 1
+		case !isBool(a) && isBool(b):
+			// bool cannot be used as variant shape
+			// See https://github.com/bytecodealliance/go-modules/issues/284
+			return -1
+		case isBool(a) && !isBool(b):
+			return 1
 		case wit.HasPointer(a) && !wit.HasPointer(b):
 			return -1
 		case !wit.HasPointer(a) && wit.HasPointer(b):
@@ -30,6 +36,16 @@ func variantShape(types []wit.Type) wit.Type {
 	return types[0]
 }
 
+func isBool(t wit.TypeDefKind) bool {
+	switch t := t.(type) {
+	case wit.Bool:
+		return true
+	case *wit.TypeDef:
+		return isBool(t.Root().Kind)
+	}
+	return false
+}
+
 // variantAlign returns the type with the largest alignment.
 func variantAlign(types []wit.Type) wit.Type {
 	if len(types) == 0 {