Skip to content

Commit 2b0ff4b

Browse files
committed
reflect: fix ArenaNew to match documentation
Currently ArenaNew expects the type passed in to be a *T and it returns a *T. This does not match the function's documentation. Since this is an experiment, change ArenaNew to match the documentation. This more closely aligns ArenaNew with arena.New. (Takes a type T, returns a *T value.) Note that this is a breaking change. However, as far as pkg.go.dev can tell, there's exactly one package using it in the open source world. Also, add smoke test for the exported API, which is just a wrapper around the internal API. Clearly there's enough room for error here that it should be tested, but we don't need thorough tests at this layer because that already exists in the runtime. We just need to make sure it basically works. Fixes #60528. Change-Id: I673cc4609378380ef80648b0c2eb2928e73f49c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/501860 Run-TryBot: Michael Knyszek <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 4eceefa commit 2b0ff4b

File tree

3 files changed

+67
-2
lines changed

3 files changed

+67
-2
lines changed

src/cmd/internal/testdir/testdir_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ var (
6464

6565
// dirs are the directories to look for *.go files in.
6666
// TODO(bradfitz): just use all directories?
67-
dirs = []string{".", "ken", "chan", "interface", "syntax", "dwarf", "fixedbugs", "codegen", "runtime", "abi", "typeparam", "typeparam/mdempsky"}
67+
dirs = []string{".", "ken", "chan", "interface", "syntax", "dwarf", "fixedbugs", "codegen", "runtime", "abi", "typeparam", "typeparam/mdempsky", "arenas"}
6868
)
6969

7070
// Test is the main entrypoint that runs tests in the GOROOT/test directory.

src/reflect/arena.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import "arena"
1212
// specified type, allocating storage for it in the provided arena. That is,
1313
// the returned Value's Type is PointerTo(typ).
1414
func ArenaNew(a *arena.Arena, typ Type) Value {
15-
return ValueOf(arena_New(a, typ))
15+
return ValueOf(arena_New(a, PointerTo(typ)))
1616
}
1717

1818
func arena_New(a *arena.Arena, typ any) any

test/arenas/smoke.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// build -goexperiment arenas
2+
3+
// Copyright 2023 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"arena"
11+
"log"
12+
"reflect"
13+
)
14+
15+
func main() {
16+
a := arena.NewArena()
17+
defer a.Free()
18+
19+
const iValue = 10
20+
21+
i := arena.New[int](a)
22+
*i = iValue
23+
24+
if *i != iValue {
25+
// This test doesn't reasonably expect this to fail. It's more likely
26+
// that *i crashes for some reason. Still, why not check it.
27+
log.Fatalf("bad i value: got %d, want %d", *i, iValue)
28+
}
29+
30+
const wantLen = 125
31+
const wantCap = 1912
32+
33+
sl := arena.MakeSlice[*int](a, wantLen, wantCap)
34+
if len(sl) != wantLen {
35+
log.Fatalf("bad arena slice length: got %d, want %d", len(sl), wantLen)
36+
}
37+
if cap(sl) != wantCap {
38+
log.Fatalf("bad arena slice capacity: got %d, want %d", cap(sl), wantCap)
39+
}
40+
sl = sl[:cap(sl)]
41+
for j := range sl {
42+
sl[j] = i
43+
}
44+
for j := range sl {
45+
if *sl[j] != iValue {
46+
// This test doesn't reasonably expect this to fail. It's more likely
47+
// that sl[j] crashes for some reason. Still, why not check it.
48+
log.Fatalf("bad sl[j] value: got %d, want %d", *sl[j], iValue)
49+
}
50+
}
51+
52+
t := reflect.TypeOf(int(0))
53+
v := reflect.ArenaNew(a, t)
54+
if want := reflect.PointerTo(t); v.Type() != want {
55+
log.Fatalf("unexpected type for arena-allocated value: got %s, want %s", v.Type(), want)
56+
}
57+
i2 := v.Interface().(*int)
58+
*i2 = iValue
59+
60+
if *i2 != iValue {
61+
// This test doesn't reasonably expect this to fail. It's more likely
62+
// that *i crashes for some reason. Still, why not check it.
63+
log.Fatalf("bad i2 value: got %d, want %d", *i2, iValue)
64+
}
65+
}

0 commit comments

Comments
 (0)