Skip to content

Commit 31d8cbc

Browse files
creastyclaude
andauthored
test: pin a caller's pointer to a slice, map or pointer as a QUIRK (#106)
Set descends into a pointer the caller allocated only when it points at a struct. A caller's *[]T, *map[K]T or **T, as a field or a slice element, is left alone: what it holds gets no defaults, a default that would fail there is not reported, and a tag on the pointer stops at it. Yet a *[]T or *map[K]T held as a map value is descended into, as it has been since v1.6.0, whose map loop follows a pointer to a struct, slice or map. Nothing pinned the skip, and the README said only that a pointer to a struct is descended into: slice := []inner{{}} // inner has Name string `default:"inner"` err := defaults.Set(&struct{ Ptr *[]inner }{Ptr: &slice}) // err=<nil>, slice[0].Name == "" The README's Maps paragraph and the Set doc, as the commit before this one wrote them, erred the other way: they said a slice, map or pointer held as a map value is filled through. A **T map value is not, since that loop follows a pointer to nothing else. No library code changes but the Set doc comment. set_pointer_test.go gains TestSet_CallerPointerToContainerIsSkipped. The README's pointer paragraph, after the sentence on a pointer to a struct, says that a pointer the caller allocated to anything else is not descended into, and that the same *[]T or *map[K]T held as a map value is, though a **T held as one is not. The Maps paragraph and the Set doc now say that no map value but a struct is stored back, and that a slice or map held as a map value is filled through, as is a pointer to a struct, slice or map, but not a pointer to anything else, such as a **T. Why this way. #100 descended into all three. The maintainer keeps the skip instead, as documented behavior. Descending walks every element behind such a pointer on each Set, which #100 measured as Walk/pointer/to_filled_slice/elements=1000 going from 163 ns to 182 us, and turns a default that fails there into an error where Set returned nil. A pointer to a pointer can also lead back to itself with no struct, slice or map between, which is all the cycle guard earlier in this stack tracks: a throwaway probe, not committed, of a type loop *loop pointing at itself passes here and overflows the stack once Set follows a caller's **T. Not a behavior change. The QUIRK comment says what is skipped, that a *[]T or *map[K]T map value is descended into since v1.6.0 (TestSet_MapOfPointerContainers), as a caller's pointer to a struct is anywhere, and links #100, closed in favor of this pin. The new test has seven subtests: a caller's *[]T, *map[string]T and **T field, a *[]T slice element and a **T map value each leave what they hold unfilled; a tag on a caller's *[]string at a nil slice leaves the slice nil and the pointer the caller's; and a default that would fail behind a caller's *[]T is not reported. All pass against the parent, whose library code this is. The whole suite, go test -race -shuffle=on -v, gives 311 PASS lines on the parent and 319 here, the 8 new ones added. With #100's library code (a341f0d) and this test added, six subtests fail, all but the tag one: #100 also kept the tag from reaching through the caller's pointer. Nothing pinned the skip, so no test is flipped. Mutation-checked, 6 of 6 killed, and under each only the new test fails, so before this commit nothing in the suite noticed any of them. Descending into a caller's pointer to a slice fails four subtests: the *[]T field, the slice element, the tag and the failing default. To a map fails the *map[K]T field, to a pointer the **T field, and to all three six, all but the **T map value. Following a pointer behind a map value's pointer, which the Maps paragraph now rules out, fails the **T map value. Carrying a tag through a caller's pointer to a slice, and descending no further, fails only the tag subtest. Coverage stays at 100.0%, and make bench-smoke passes all 45 cases. Replaces #100, which closes in favor of this pin. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 6852e14 commit 31d8cbc

3 files changed

Lines changed: 109 additions & 5 deletions

File tree

README.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ default replaces it.
7676
Use a pointer where that distinction matters. `nil` means unspecified, and a pointer to a zero
7777
scalar is preserved: `*bool` is the way to let `false` survive a `default:"true"`. A pointer to a
7878
struct is descended into like the struct itself, though, so the struct's zero fields still get
79-
their defaults.
79+
their defaults. A pointer the caller allocated to anything else is not: neither its tag nor the walk
80+
goes past it, so the elements behind a caller's `*[]T` or `*map[K]T`, and the struct behind a `**T`,
81+
get no defaults, and a default that would fail there is not reported. The same `*[]T` or `*map[K]T`
82+
held as a map value is descended into, though a `**T` held as one is not.
8083

8184
```go
8285
type Feature struct {
@@ -136,8 +139,9 @@ still being walked, `Set` goes no further, and the walk already under way finish
136139
A struct held as a map value cannot be filled in place, so `Set` fills a copy and stores it back
137140
under its key: every such struct it walks, whether or not anything changed. That store is a write to
138141
the map, so do not call `Set` while another goroutine reads a map of structs it walks, even one with
139-
nothing left to fill. A slice, map or pointer held as a map value is filled through, and never
140-
stored back, so `Set` only reads the map holding it.
142+
nothing left to fill. No other map value is stored back, so `Set` only reads a map of anything else.
143+
A slice or map held as a map value is filled through, as is a pointer to a struct, slice or map; a
144+
pointer to anything else, such as a `**T`, is not descended into.
141145

142146
### Errors
143147

set.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,9 @@ func (w *walking) repeats() bool {
7676
// value still being walked, Set goes no further, so a cycle ends.
7777
//
7878
// A struct held as a map value is not addressable, so Set fills a copy and stores it back under its
79-
// key whether or not anything changed: no other goroutine may read that map while Set runs. A slice,
80-
// map or pointer held as a map value is filled through, and not stored back.
79+
// key whether or not anything changed: no other goroutine may read that map while Set runs. No
80+
// other map value is stored back. A slice or map held as a map value is filled through, and so is
81+
// a pointer to a struct, slice or map, but a pointer to anything else, such as a **T, is not.
8182
//
8283
// Set stops at the first field whose default fails and returns its error. A value Set found zero on
8384
// the way to that default is left zero again, so a second Set fails again; fields filled elsewhere

set_pointer_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,105 @@ func TestSet_UntaggedPointerStructRecurses(t *testing.T) {
138138
assert.Equal(t, child{Name: "Jim", Age: 20}, *got.Child, "the caller's name survives, the missing age is filled")
139139
}
140140

141+
// TestSet_CallerPointerToContainerIsSkipped covers a pointer the caller allocated to a slice, a map or
142+
// a pointer, as a field, a slice element and a map value.
143+
//
144+
// QUIRK: Set does not descend into such a pointer, so what a caller's *[]T, *map[K]T or **T holds
145+
// gets no defaults, a default that would fail there is not reported, and a tag on the pointer stops
146+
// at it. Yet a *[]T or *map[K]T held as a map value is descended into, and has been since v1.6.0
147+
// (TestSet_MapOfPointerContainers), as a caller's pointer to a struct is anywhere
148+
// (TestSet_UntaggedPointerStructRecurses). https://github.com/creasty/defaults/pull/100 descended
149+
// into all three and was closed in favor of this pin: that walks every element behind such a
150+
// pointer on each Set, and a pointer to a pointer can lead back to itself with no struct, slice or
151+
// map between, which is all the cycle guard tracks.
152+
func TestSet_CallerPointerToContainerIsSkipped(t *testing.T) {
153+
type inner struct {
154+
Name string `default:"inner"`
155+
}
156+
157+
t.Run("a *[]T field", func(t *testing.T) {
158+
slice := []inner{{}}
159+
got := struct {
160+
Ptr *[]inner
161+
}{Ptr: &slice}
162+
163+
require.NoError(t, defaults.Set(&got))
164+
165+
assert.Empty(t, slice[0].Name)
166+
})
167+
168+
t.Run("a *map[K]T field", func(t *testing.T) {
169+
mapping := map[string]inner{"a": {}}
170+
got := struct {
171+
Ptr *map[string]inner
172+
}{Ptr: &mapping}
173+
174+
require.NoError(t, defaults.Set(&got))
175+
176+
assert.Empty(t, mapping["a"].Name)
177+
})
178+
179+
t.Run("a **T field", func(t *testing.T) {
180+
ptr := &inner{}
181+
got := struct {
182+
Ptr **inner
183+
}{Ptr: &ptr}
184+
185+
require.NoError(t, defaults.Set(&got))
186+
187+
assert.Empty(t, ptr.Name)
188+
})
189+
190+
t.Run("a *[]T slice element", func(t *testing.T) {
191+
slice := []inner{{}}
192+
got := struct {
193+
Slices []*[]inner
194+
}{Slices: []*[]inner{&slice}}
195+
196+
require.NoError(t, defaults.Set(&got))
197+
198+
assert.Empty(t, slice[0].Name)
199+
})
200+
201+
t.Run("a **T map value", func(t *testing.T) {
202+
ptr := &inner{}
203+
got := struct {
204+
Map map[string]**inner
205+
}{Map: map[string]**inner{"a": &ptr}}
206+
207+
require.NoError(t, defaults.Set(&got))
208+
209+
assert.Empty(t, ptr.Name)
210+
})
211+
212+
t.Run("a tag on a *[]T field", func(t *testing.T) {
213+
var slice []string
214+
got := struct {
215+
Ptr *[]string `default:"[\"tag\"]"`
216+
}{Ptr: &slice}
217+
218+
require.NoError(t, defaults.Set(&got))
219+
220+
assert.Same(t, &slice, got.Ptr)
221+
assert.Nil(t, slice, "the tag does not reach the caller's nil slice")
222+
})
223+
224+
t.Run("a default that would fail behind a *[]T field", func(t *testing.T) {
225+
type bad struct {
226+
Ints []int `default:"[!]"`
227+
}
228+
229+
slice := []bad{{}}
230+
got := struct {
231+
Ptr *[]bad
232+
}{Ptr: &slice}
233+
234+
require.NoError(t, defaults.Set(&got))
235+
236+
assert.Nil(t, slice[0].Ints)
237+
})
238+
}
239+
141240
// TestSet_PointerWithEmptyTag covers `default:""` on a pointer. The tag is present, so the pointer
142241
// is allocated and points at the zero value — it used to stay nil, which left no way to ask for a
143242
// pointer to an empty string. See https://github.com/creasty/defaults/issues/52.

0 commit comments

Comments
 (0)