Skip to content

Commit 9a4e139

Browse files
ydnarydnar
committed
cm: BREAKING: change BoolResult from bool to uint8
Fixes issue where LLVM clobbers high 7 bits in a bool value when used as a shape for a variant, result, or option. Fixes #344.
1 parent 0e176bd commit 9a4e139

7 files changed

Lines changed: 202 additions & 6 deletions

File tree

‎cm/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66

77
### Added
88

9+
- Breaking: `BoolResult` is now represented as a `uint8` rather than a `bool`. This fixes an issue with TinyGo where `bool` values are treated distinctly from `uint8`. See [#344](https://github.com/bytecodealliance/go-modules/issues/344) for more information.
910
- Mutating methods `SetOK` and `SetErr` on `result` types (`Result[Shape, OK, Err]`).
1011

12+
### Fixed
13+
14+
- [#344](https://github.com/bytecodealliance/go-modules/issues/344): the memory representation of `option`, `result` now use `uint8` instead of `bool` for the discriminator. LLVM optimizes `bool` values into a single bit, which breaks WIT variants where the associated types share memory.
15+
1116
## [v0.2.2] — 2025-03-16
1217

1318
### Fixed

‎cm/result.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ import "unsafe"
44

55
const (
66
// ResultOK represents the OK case of a result.
7-
ResultOK = false
7+
ResultOK = 0
88

99
// ResultErr represents the error case of a result.
10-
ResultErr = true
10+
ResultErr = 1
1111
)
1212

1313
// BoolResult represents a result with no OK or error type.
1414
// False represents the OK case and true represents the error case.
15-
type BoolResult bool
15+
type BoolResult uint8
1616

1717
// Result represents a result sized to hold the Shape type.
1818
// The size of the Shape type must be greater than or equal to the size of OK and Err types.

‎cm/result_test.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestResultLayout(t *testing.T) {
8787
size uintptr
8888
offset uintptr
8989
}{
90-
{"result", BoolResult(false), 1, 0},
90+
{"result", BoolResult(0), 1, 0},
9191
{"ok", BoolResult(ResultOK), 1, 0},
9292
{"err", BoolResult(ResultErr), 1, 0},
9393

@@ -375,3 +375,15 @@ func TestIssue344TupleOfResult(t *testing.T) {
375375
t.Errorf("*v.OK(): %v, expected %v", got, want)
376376
}
377377
}
378+
379+
func TestIssue344BoolResult(t *testing.T) {
380+
type T Result[BoolResult, uint8, BoolResult]
381+
382+
want := uint8(2)
383+
v := T(OK[T](want))
384+
got := *v.OK()
385+
386+
if got != want {
387+
t.Errorf("*v.OK(): %v, expected %v", got, want)
388+
}
389+
}

‎testdata/issues/issue344.wit‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package issues:issue344;
2+
3+
world w {
4+
import i;
5+
}
6+
7+
interface i {
8+
type a = result<u64, tuple<option<u64>>>;
9+
type b = result<u64, tuple<result<u64, bool>>>;
10+
type c = result<result, u8>;
11+
12+
fa: func() -> a;
13+
fb: func() -> b;
14+
fc: func() -> c;
15+
}

‎testdata/issues/issue344.wit.json‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
{
2+
"worlds": [
3+
{
4+
"name": "w",
5+
"imports": {
6+
"interface-0": {
7+
"interface": {
8+
"id": 0
9+
}
10+
}
11+
},
12+
"exports": {},
13+
"package": 0
14+
}
15+
],
16+
"interfaces": [
17+
{
18+
"name": "i",
19+
"types": {
20+
"a": 2,
21+
"b": 5,
22+
"c": 7
23+
},
24+
"functions": {
25+
"fa": {
26+
"name": "fa",
27+
"kind": "freestanding",
28+
"params": [],
29+
"result": 2
30+
},
31+
"fb": {
32+
"name": "fb",
33+
"kind": "freestanding",
34+
"params": [],
35+
"result": 5
36+
},
37+
"fc": {
38+
"name": "fc",
39+
"kind": "freestanding",
40+
"params": [],
41+
"result": 7
42+
}
43+
},
44+
"package": 0
45+
}
46+
],
47+
"types": [
48+
{
49+
"name": null,
50+
"kind": {
51+
"option": "u64"
52+
},
53+
"owner": null
54+
},
55+
{
56+
"name": null,
57+
"kind": {
58+
"tuple": {
59+
"types": [
60+
0
61+
]
62+
}
63+
},
64+
"owner": null
65+
},
66+
{
67+
"name": "a",
68+
"kind": {
69+
"result": {
70+
"ok": "u64",
71+
"err": 1
72+
}
73+
},
74+
"owner": {
75+
"interface": 0
76+
}
77+
},
78+
{
79+
"name": null,
80+
"kind": {
81+
"result": {
82+
"ok": "u64",
83+
"err": "bool"
84+
}
85+
},
86+
"owner": null
87+
},
88+
{
89+
"name": null,
90+
"kind": {
91+
"tuple": {
92+
"types": [
93+
3
94+
]
95+
}
96+
},
97+
"owner": null
98+
},
99+
{
100+
"name": "b",
101+
"kind": {
102+
"result": {
103+
"ok": "u64",
104+
"err": 4
105+
}
106+
},
107+
"owner": {
108+
"interface": 0
109+
}
110+
},
111+
{
112+
"name": null,
113+
"kind": {
114+
"result": {
115+
"ok": null,
116+
"err": null
117+
}
118+
},
119+
"owner": null
120+
},
121+
{
122+
"name": "c",
123+
"kind": {
124+
"result": {
125+
"ok": 6,
126+
"err": "u8"
127+
}
128+
},
129+
"owner": {
130+
"interface": 0
131+
}
132+
}
133+
],
134+
"packages": [
135+
{
136+
"name": "issues:issue344",
137+
"interfaces": {
138+
"i": 0
139+
},
140+
"worlds": {
141+
"w": 0
142+
}
143+
}
144+
]
145+
}

‎testdata/issues/issue344.wit.json.golden.wit‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package issues:issue344;
2+
3+
interface i {
4+
type a = result<u64, tuple<option<u64>>>;
5+
type b = result<u64, tuple<result<u64, bool>>>;
6+
type c = result<result, u8>;
7+
fa: func() -> a;
8+
fb: func() -> b;
9+
fc: func() -> c;
10+
}
11+
12+
world w {
13+
import i;
14+
}

‎wit/bindgen/generator.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,11 @@ func (g *generator) typeDefShape(file *gen.File, dir wit.Direction, t *wit.TypeD
10361036
// Variants that can be represented as an enum do not need a custom shape.
10371037
return g.typeRep(file, dir, t)
10381038
}
1039+
case *wit.Result:
1040+
if len(kind.Types()) == 0 {
1041+
// Results without associated types do not need a custom shape.
1042+
return g.typeRep(file, dir, t)
1043+
}
10391044
case *wit.Tuple:
10401045
if kind.Type() != nil {
10411046
// Monotypic tuples have a packed memory layout.
@@ -1214,7 +1219,7 @@ func (g *generator) lowerVariant(file *gen.File, dir wit.Direction, t *wit.TypeD
12141219
func (g *generator) lowerResult(file *gen.File, dir wit.Direction, t *wit.TypeDef, input string) string {
12151220
r := t.Kind.(*wit.Result)
12161221
if r.OK == nil && r.Err == nil {
1217-
return g.cast(file, dir, wit.Bool{}, wit.U32{}, input)
1222+
return g.cast(file, dir, wit.U8{}, wit.U32{}, input)
12181223
}
12191224
flat := t.Flat()
12201225
abiFile := g.abiFile(file.Package)
@@ -1425,7 +1430,7 @@ func (g *generator) liftResult(file *gen.File, dir wit.Direction, t *wit.TypeDef
14251430
r := t.Kind.(*wit.Result)
14261431
flat := t.Flat()
14271432
if r.OK == nil && r.Err == nil {
1428-
return g.cast(file, dir, wit.Bool{}, t, g.cast(file, dir, flat[0], wit.Bool{}, input))
1433+
return g.cast(file, dir, wit.Bool{}, t, g.cast(file, dir, flat[0], wit.U8{}, input))
14291434
}
14301435
abiFile := g.abiFile(file.Package)
14311436
var b strings.Builder

0 commit comments

Comments
 (0)