diff --git a/lib/tag/tag.go b/lib/tag/tag.go index 35eef7d3..7409bd07 100644 --- a/lib/tag/tag.go +++ b/lib/tag/tag.go @@ -83,12 +83,16 @@ func sameActiveNode(a, b any) bool { // Value.Pointer identifies shared function code, not closure identity, so it // cannot detect cycles. maxTagDepth still bounds recursive function getters. return false - case reflect.Pointer, reflect.Slice, reflect.Map, reflect.Chan, reflect.UnsafePointer: - // For slices, Pointer() is the address of the first element, so two - // distinct slices aliasing the same backing array at the same start - // compare equal here. That can only over-detect a cycle and truncate - // expansion early, which maxTagCount/maxTagDepth already bound; real tag - // data does not take that shape. + case reflect.Slice: + // Value.Pointer for a slice is the address of its first element, so two + // distinct views into the same backing array share it. A slice header is + // fully identified by its pointer, length and capacity, so compare all + // three: views differing in length or capacity are distinct nodes (a named + // slice TagGetter can observe its capacity via cap and yield different + // tags), while a genuine self-referential slice re-enters with an identical + // header and is still detected as a cycle. + return va.Pointer() == vb.Pointer() && va.Len() == vb.Len() && va.Cap() == vb.Cap() + case reflect.Pointer, reflect.Map, reflect.Chan, reflect.UnsafePointer: return va.Pointer() == vb.Pointer() default: return false diff --git a/lib/tag/tag_test.go b/lib/tag/tag_test.go index 5a00c3f8..cd7c59cb 100644 --- a/lib/tag/tag_test.go +++ b/lib/tag/tag_test.go @@ -60,6 +60,20 @@ func (tt *testMutualSliceTagger) JawsGetTag(Context) any { return []any{tt.next} } +// testCapTagger is a named slice type whose tag depends on its capacity: a view +// with spare capacity yields Tag("outer") plus a capacity-trimmed view of +// itself, which (having no spare capacity) yields Tag("inner"). The two views +// share a first-element pointer and length but differ in capacity, so they must +// be treated as distinct active nodes. +type testCapTagger []int + +func (c testCapTagger) JawsGetTag(Context) any { + if cap(c) > len(c) { + return []any{Tag("outer"), c[:len(c):len(c)]} + } + return Tag("inner") +} + type testTagExpandNestedTagGetter struct { Setter testNestedTagGetter Vals []int @@ -375,6 +389,45 @@ func TestTagExpand_SelfReferentialSliceStopsRecursing(t *testing.T) { } } +// TestTagExpand_AliasedSliceViews reproduces #203: two slices sharing a backing +// array at the same start but with different lengths must not be conflated as a +// cycle. Expanding the shorter view whose element references the longer backing +// slice must still visit every tag reachable through the longer slice. +func TestTagExpand_AliasedSliceViews(t *testing.T) { + all := make([]any, 3) + outer := all[:2] + + all[0] = Tag("first") + all[1] = all + all[2] = Tag("last") + + got, err := TagExpand(nil, outer) + if err != nil { + t.Fatal(err) + } + want := []any{Tag("first"), Tag("last")} + if !reflect.DeepEqual(got, want) { + t.Fatalf("TagExpand() = %#v, want %#v", got, want) + } +} + +// TestTagExpand_CapacityDependentSliceGetter guards against conflating two +// aliased slice-typed TagGetters that share a first-element pointer and length +// but differ in capacity. Identifying active nodes by pointer and length alone +// would treat the capacity-trimmed inner view as a cycle back to the outer view +// and, because a non-comparable slice getter cannot itself be a tag key, fail +// with ErrNotUsableAsTag instead of invoking the inner getter. +func TestTagExpand_CapacityDependentSliceGetter(t *testing.T) { + backing := make(testCapTagger, 2) + outer := backing[:1] // len 1, cap 2: yields Tag("outer") and a cap-1 inner view + + got, err := TagExpand(nil, outer) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, got, []any{Tag("outer"), Tag("inner")}) +} + func TestTagExpand_TooManyTagsPanic(t *testing.T) { tags := make([]any, 101) for i := range tags { @@ -722,6 +775,25 @@ func TestSameActiveNode_NilAndDefaultCases(t *testing.T) { } } +func TestSameActiveNode_AliasedSliceViews(t *testing.T) { + backing := []any{Tag("a"), Tag("b"), Tag("c")} + short := backing[:2] + // Same start pointer, different lengths: distinct traversal nodes (#203). + if sameActiveNode(short, backing) { + t.Fatal("expected aliased slice views of different lengths not to match") + } + // Same start pointer and length, different capacities: also distinct nodes, + // since a named slice getter can observe capacity and produce different tags. + if sameActiveNode(backing[:2], backing[:2:2]) { + t.Fatal("expected aliased slice views of different capacities not to match") + } + // Identical start pointer, length and capacity: the same node, so a genuine + // self-referential slice is still detected as a cycle. + if !sameActiveNode(backing, backing) { + t.Fatal("expected a slice to match itself") + } +} + func TestTagExpand_TooDeepAndTooManySliceTags(t *testing.T) { var nested any = Tag("leaf") for range 11 {