Skip to content

Commit 02663f3

Browse files
nevkontaktenevkontakte
committed
Support go1.17 slice to array pointer conversion.
Conversion is fully supported for slices of numeric types, which is probably the most common use case for this feature judging by the discussion in the proposal. However, it is only partially supported for slices of complex types (e.g. strings), since they are backed by the JavaScript's built-in Array type, which lacks ability to share backing memory among subarrays. Conversion works in cases when the slice is converted into array that exactly matches the slice's underlying array, but panics if we are trying to convert a subslice. I feel like an explicit failure is better than a chance of introducing subtle bugs. It also seems that GopherJS internally doesn't really distinguish between array types and pointer to array types, which makes the whole implementation somewhat messy when it comes to nil vs non-nil pointers to arrays. Last but not least, I've moved pointer cache for numeric arrays into the backing ArrayBuffer, which ensures that pointers are comparable between different subarrays.
1 parent 9680b1b commit 02663f3

File tree

6 files changed

+208
-62
lines changed

6 files changed

+208
-62
lines changed

‎compiler/expressions.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -426,10 +426,6 @@ func (fc *funcContext) translateExpr(expr ast.Expr) *expression {
426426
return fc.formatExpr("$equal(%e, %e, %s)", e.X, e.Y, fc.typeName(t))
427427
case *types.Interface:
428428
return fc.formatExpr("$interfaceIsEqual(%s, %s)", fc.translateImplicitConversion(e.X, t), fc.translateImplicitConversion(e.Y, t))
429-
case *types.Pointer:
430-
if _, ok := u.Elem().Underlying().(*types.Array); ok {
431-
return fc.formatExpr("$equal(%s, %s, %s)", fc.translateImplicitConversion(e.X, t), fc.translateImplicitConversion(e.Y, t), fc.typeName(u.Elem()))
432-
}
433429
case *types.Basic:
434430
if isBoolean(u) {
435431
if b, ok := analysis.BoolValue(e.X, fc.pkgCtx.Info.Info); ok && b {
@@ -1031,7 +1027,7 @@ func (fc *funcContext) translateConversion(expr ast.Expr, desiredType types.Type
10311027
case t.Kind() == types.UnsafePointer:
10321028
if unary, isUnary := expr.(*ast.UnaryExpr); isUnary && unary.Op == token.AND {
10331029
if indexExpr, isIndexExpr := unary.X.(*ast.IndexExpr); isIndexExpr {
1034-
return fc.formatExpr("$sliceToArray(%s)", fc.translateConversionToSlice(indexExpr.X, types.NewSlice(types.Typ[types.Uint8])))
1030+
return fc.formatExpr("$sliceToNativeArray(%s)", fc.translateConversionToSlice(indexExpr.X, types.NewSlice(types.Typ[types.Uint8])))
10351031
}
10361032
if ident, isIdent := unary.X.(*ast.Ident); isIdent && ident.Name == "_zero" {
10371033
return fc.formatExpr("new Uint8Array(0)")
@@ -1075,8 +1071,14 @@ func (fc *funcContext) translateConversion(expr ast.Expr, desiredType types.Type
10751071
break
10761072
}
10771073

1078-
switch u := t.Elem().Underlying().(type) {
1074+
switch ptrElType := t.Elem().Underlying().(type) {
10791075
case *types.Array: // (*[N]T)(expr) — converting expr to a pointer to an array.
1076+
if _, ok := exprType.Underlying().(*types.Slice); ok {
1077+
// GopherJS interprets pointer to an array as the array object itself
1078+
// due to its reference semantics, so the bellow coversion is correct.
1079+
return fc.formatExpr("$sliceToGoArray(%e, %s)", expr, fc.typeName(t))
1080+
}
1081+
// TODO(nevkontakte): Is this just for aliased types (e.g. `type a [4]byte`)?
10801082
return fc.translateExpr(expr)
10811083
case *types.Struct: // (*StructT)(expr) — converting expr to a pointer to a struct.
10821084
if fc.pkgCtx.Pkg.Path() == "syscall" && types.Identical(exprType, types.Typ[types.UnsafePointer]) {
@@ -1086,7 +1088,7 @@ func (fc *funcContext) translateConversion(expr ast.Expr, desiredType types.Type
10861088
// indeed pointing at a byte array.
10871089
array := fc.newVariable("_array")
10881090
target := fc.newVariable("_struct")
1089-
return fc.formatExpr("(%s = %e, %s = %e, %s, %s)", array, expr, target, fc.zeroValue(t.Elem()), fc.loadStruct(array, target, u), target)
1091+
return fc.formatExpr("(%s = %e, %s = %e, %s, %s)", array, expr, target, fc.zeroValue(t.Elem()), fc.loadStruct(array, target, ptrElType), target)
10901092
}
10911093
// Convert between structs of different types but identical layouts,
10921094
// for example:

‎compiler/prelude/jsmapping.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ var $externalize = function(v, t) {
7474
return $externalize(v.$get(), t.elem);
7575
case $kindSlice:
7676
if ($needsExternalization(t.elem)) {
77-
return $mapArray($sliceToArray(v), function(e) { return $externalize(e, t.elem); });
77+
return $mapArray($sliceToNativeArray(v), function(e) { return $externalize(e, t.elem); });
7878
}
79-
return $sliceToArray(v);
79+
return $sliceToNativeArray(v);
8080
case $kindString:
8181
if ($isASCII(v)) {
8282
return v;

‎compiler/prelude/prelude.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,41 @@ var $substring = function(str, low, high) {
145145
return str.substring(low, high);
146146
};
147147
148-
var $sliceToArray = function(slice) {
148+
// Convert Go slice to an equivalent JS array type.
149+
var $sliceToNativeArray = function(slice) {
149150
if (slice.$array.constructor !== Array) {
150151
return slice.$array.subarray(slice.$offset, slice.$offset + slice.$length);
151152
}
152153
return slice.$array.slice(slice.$offset, slice.$offset + slice.$length);
153154
};
154155
156+
// Convert Go slice to a pointer to an underlying Go array.
157+
var $sliceToGoArray = function(slice, arrayPtrType) {
158+
var arrayType = arrayPtrType.elem;
159+
if (arrayType !== undefined && slice.$length < arrayType.len) {
160+
$throwRuntimeError("cannot convert slice with length " + slice.$length + " to pointer to array with length " + arrayType.len);
161+
}
162+
if (slice == slice.constructor.nil) {
163+
return arrayPtrType.nil; // Nil slice converts to nil array pointer.
164+
}
165+
if (slice.$array.constructor !== Array) {
166+
return slice.$array.subarray(slice.$offset, slice.$offset + slice.$length);
167+
}
168+
if (slice.$offset == 0 && slice.$length == slice.$capacity && slice.$length == arrayType.len) {
169+
return slice.$array;
170+
}
171+
if (arrayType.len == 0) {
172+
return new arrayType([]);
173+
}
174+
175+
// Array.slice (unlike TypedArray.subarray) returns a copy of an array range,
176+
// which is not sharing memory with the original one, which violates the spec
177+
// for slice to array conversion. This is incompatible with the Go spec, in
178+
// particular that the assignments to the array elements would be visible in
179+
// the slice. Prefer to fail explicitly instead of creating subtle bugs.
180+
$throwRuntimeError("gopherjs: non-numeric slice to underlying array conversion is not supported for subslices");
181+
};
182+
155183
var $decodeRune = function(str, pos) {
156184
var c0 = str.charCodeAt(pos);
157185

‎compiler/prelude/prelude_min.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎compiler/prelude/types.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,17 @@ var $newDataPointer = function(data, constructor) {
637637
};
638638
639639
var $indexPtr = function(array, index, constructor) {
640-
array.$ptr = array.$ptr || {};
641-
return array.$ptr[index] || (array.$ptr[index] = new constructor(function() { return array[index]; }, function(v) { array[index] = v; }));
640+
if (array.buffer) {
641+
// Pointers to the same underlying ArrayBuffer share cache.
642+
var cache = array.buffer.$ptr = array.buffer.$ptr || {};
643+
// Pointers of different primitive types are non-comparable and stored in different caches.
644+
var typeCache = cache[array.name] = cache[array.name] || {};
645+
var cacheIdx = array.BYTES_PER_ELEMENT * index + array.byteOffset;
646+
return typeCache[cacheIdx] || (typeCache[cacheIdx] = new constructor(function() { return array[index]; }, function(v) { array[index] = v; }));
647+
} else {
648+
array.$ptr = array.$ptr || {};
649+
return array.$ptr[index] || (array.$ptr[index] = new constructor(function() { return array[index]; }, function(v) { array[index] = v; }));
650+
}
642651
};
643652
644653
var $sliceType = function(elem) {

‎tests/slice_to_array_ptr_test.go

Lines changed: 156 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,161 @@ package tests
22

33
import "testing"
44

5+
// https://tip.golang.org/ref/spec#Conversions_from_slice_to_array_pointer
56
func TestSliceToArrayPointerConversion(t *testing.T) {
6-
// https://tip.golang.org/ref/spec#Conversions_from_slice_to_array_pointer
7-
s := make([]byte, 2, 4)
8-
s0 := (*[0]byte)(s)
9-
if s0 == nil {
10-
t.Error("s0 should not be nil")
11-
}
12-
s2 := (*[2]byte)(s)
13-
if &s2[0] != &s[0] {
14-
t.Error("&s2[0] should match &s[0]")
15-
}
16-
r := func() (r interface{}) {
17-
defer func() {
18-
r = recover()
19-
}()
20-
s4 := (*[4]byte)(s)
21-
_ = s4
22-
return nil
23-
}()
24-
if r == nil {
25-
t.Error("out-of-bounds conversion of s should panic")
26-
}
27-
28-
(*s2)[0] = 'x'
29-
if s[0] != 'x' {
30-
t.Errorf("s[0] should be changed")
31-
}
32-
33-
var q []string
34-
q0 := (*[0]string)(q)
35-
if q0 != nil {
36-
t.Error("t0 should be nil")
37-
}
38-
r = func() (r interface{}) {
39-
defer func() {
40-
r = recover()
41-
}()
42-
q1 := (*[1]string)(q)
43-
_ = q1
44-
return nil
45-
}
46-
if r == nil {
47-
t.Error("out-of-bounds conversion of q should panic")
48-
}
49-
50-
u := make([]byte, 0)
51-
u0 := (*[0]byte)(u)
52-
if u0 == nil {
53-
t.Error("u0 should not be nil")
54-
}
7+
// GopherJS uses TypedArray for numeric types and Array for everything else
8+
// since those are substantially different types, the tests are repeated
9+
// for both.
10+
t.Run("Numeric", func(t *testing.T) {
11+
s := make([]byte, 2, 4)
12+
t.Run("NotNil", func(t *testing.T) {
13+
s0 := (*[0]byte)(s)
14+
if s0 == nil {
15+
t.Error("s0 should not be nil")
16+
}
17+
})
18+
19+
t.Run("ElementPointerEquality", func(t *testing.T) {
20+
s2 := (*[2]byte)(s)
21+
if &s2[0] != &s[0] {
22+
t.Error("&s2[0] should match &s[0]")
23+
}
24+
s3 := (*[1]byte)(s[1:])
25+
if &s3[0] != &s[1] {
26+
t.Error("&s3[0] should match &s[1]")
27+
}
28+
})
29+
30+
t.Run("SliceToLargerArray", func(t *testing.T) {
31+
r := func() (r interface{}) {
32+
defer func() { r = recover() }()
33+
s4 := (*[4]byte)(s)
34+
_ = s4
35+
return nil
36+
}()
37+
if r == nil {
38+
t.Error("out-of-bounds conversion of s should panic")
39+
}
40+
})
41+
42+
t.Run("SharedMemory", func(t *testing.T) {
43+
s2 := (*[2]byte)(s)
44+
(*s2)[0] = 'x'
45+
if s[0] != 'x' {
46+
t.Errorf("s[0] should be changed")
47+
}
48+
49+
s3 := (*[1]byte)(s[1:])
50+
(*s3)[0] = 'y'
51+
if s[1] != 'y' {
52+
t.Errorf("s[1] should be changed")
53+
}
54+
})
55+
56+
var q []byte
57+
t.Run("NilSlice", func(t *testing.T) {
58+
q0 := (*[0]byte)(q)
59+
if q0 != nil {
60+
t.Error("q0 should be nil")
61+
}
62+
})
63+
64+
t.Run("NilSliceToLargerArray", func(t *testing.T) {
65+
r := func() (r interface{}) {
66+
defer func() { r = recover() }()
67+
q1 := (*[1]byte)(q)
68+
_ = q1
69+
return nil
70+
}
71+
if r == nil {
72+
t.Error("out-of-bounds conversion of q should panic")
73+
}
74+
})
75+
76+
t.Run("ZeroLenSlice", func(t *testing.T) {
77+
u := make([]byte, 0)
78+
u0 := (*[0]byte)(u)
79+
if u0 == nil {
80+
t.Error("u0 should not be nil")
81+
}
82+
})
83+
})
84+
85+
t.Run("String", func(t *testing.T) {
86+
s := make([]string, 2, 2)
87+
t.Run("NotNil", func(t *testing.T) {
88+
s0 := (*[0]string)(s)
89+
if s0 == nil {
90+
t.Error("s0 should not be nil")
91+
}
92+
})
93+
94+
t.Run("ElementPointerEquality", func(t *testing.T) {
95+
s2 := (*[2]string)(s)
96+
if &s2[0] != &s[0] {
97+
t.Error("&s2[0] should match &s[0]")
98+
}
99+
100+
t.Skip("non-numeric slice to underlying array conversion is not supported for subslices")
101+
s3 := (*[1]string)(s[1:])
102+
if &s3[0] != &s[1] {
103+
t.Error("&s3[0] should match &s[1]")
104+
}
105+
})
106+
107+
t.Run("SliceToLargerArray", func(t *testing.T) {
108+
r := func() (r interface{}) {
109+
defer func() { r = recover() }()
110+
s4 := (*[4]string)(s)
111+
_ = s4
112+
return nil
113+
}()
114+
if r == nil {
115+
t.Error("out-of-bounds conversion of s should panic")
116+
}
117+
})
118+
119+
t.Run("SharedMemory", func(t *testing.T) {
120+
s2 := (*[2]string)(s)
121+
(*s2)[0] = "x"
122+
if s[0] != "x" {
123+
t.Errorf("s[0] should be changed")
124+
}
125+
126+
t.Skip("non-numeric slice to underlying array conversion is not supported for subslices")
127+
s3 := (*[1]string)(s[1:])
128+
(*s3)[0] = "y"
129+
if s[1] != "y" {
130+
t.Errorf("s[1] should be changed")
131+
}
132+
})
133+
134+
var q []string
135+
t.Run("NilSlice", func(t *testing.T) {
136+
q0 := (*[0]string)(q)
137+
if q0 != nil {
138+
t.Error("q0 should be nil")
139+
}
140+
})
141+
142+
t.Run("NilSliceToLargerArray", func(t *testing.T) {
143+
r := func() (r interface{}) {
144+
defer func() { r = recover() }()
145+
q1 := (*[1]string)(q)
146+
_ = q1
147+
return nil
148+
}
149+
if r == nil {
150+
t.Error("out-of-bounds conversion of q should panic")
151+
}
152+
})
153+
154+
t.Run("ZeroLenSlice", func(t *testing.T) {
155+
u := make([]string, 0)
156+
u0 := (*[0]string)(u)
157+
if u0 == nil {
158+
t.Error("u0 should not be nil")
159+
}
160+
})
161+
})
55162
}

0 commit comments

Comments
 (0)