Skip to content

fix: expand aliased slice views with differing lengths in TagExpand#214

Merged
linkdata merged 2 commits into
mainfrom
fix/tagexpand-aliased-slice-views-203
Jul 22, 2026
Merged

fix: expand aliased slice views with differing lengths in TagExpand#214
linkdata merged 2 commits into
mainfrom
fix/tagexpand-aliased-slice-views-203

Conversation

@linkdata

Copy link
Copy Markdown
Owner

Fixes #203.

Problem

TagExpand's cycle detector (sameActiveNode) identified an active []any
node by its first-element pointer alone. For a slice, reflect.Value.Pointer()
is the address of the first element, so two distinct slice headers viewing the
same backing array at the same start compared equal even with different lengths.

Expanding a short view whose element references the longer backing slice then
mistook the longer slice for a cycle back to the short view, skipped its
expansion, and silently dropped every later tag:

all := make([]any, 3)
outer := all[:2]
all[0] = tag.Tag("first")
all[1] = all
all[2] = tag.Tag("last")
tag.TagExpand(nil, outer) // returned {"first"}; "last" dropped with no error

Downstream Dirty/broadcast/lookup on the dropped tag then silently miss those
Elements.

Fix

Give reflect.Slice its own case in sameActiveNode that also compares length:

  • Views of differing lengths iterate different elements, so they are distinct
    traversal nodes.
  • A genuine self-referential slice re-enters with an identical pointer and
    length and is still detected as a cycle.

Capacity is intentionally not compared — two views with the same start and
length iterate identical elements, so they are correctly the same node. The
change only makes detection more precise (removes a false positive); even a
hypothetical missed cycle stays bounded by maxTagDepth/maxTagCount.

Tests

  • TestTagExpand_AliasedSliceViews — the issue's exact reproduction.
  • TestSameActiveNode_AliasedSliceViews — direct helper coverage of the
    differing-length (distinct) and self (same-node) cases.

Both fail without the fix and pass with it. Existing cycle tests
(TestTagExpand_SelfReferentialSliceStopsRecursing, the TagGetter self/mutual
cycle tests) stay green.

Verified with gofmt -l, go vet ./lib/tag/, and go test -race ./...
(all packages green, no races).

Known follow-up (out of scope)

FindTagGetter in lib/tag/errnotusableastag.go has an analogous
seenPtr{t, ptr} slice-aliasing collision, but it only powers a best-effort
error-message hint — over-detection there degrades a hint string, never drops
real tags. Left untouched to keep this change focused on the reported bug.

linkdata added 2 commits July 22, 2026 21:44
sameActiveNode identified an active slice node by its first-element
pointer alone. Two slices viewing the same backing array at the same
start (e.g. s[:2] and s[:3]) therefore compared equal, so a nested
longer slice was mistaken for a cycle back to a shorter view and its
later elements were silently dropped from the expansion.

Give reflect.Slice its own case that also compares length: views of
differing lengths are distinct traversal nodes, while a genuine
self-referential slice re-enters with an identical pointer and length
and is still detected as a cycle.

Fixes #203
sameActiveNode compared slice nodes by first-element pointer and length.
A named slice type implementing JawsGetTag can observe its capacity via
cap, so two views sharing a pointer and length but differing in capacity
are distinct getters that may yield different tags. Comparing pointer and
length alone falsely reported a cycle and, because a non-comparable slice
getter cannot itself be a tag key, returned ErrNotUsableAsTag instead of
invoking the second getter.

Compare pointer, length and capacity, which together fully identify a
slice header. A genuine self-referential slice re-enters with an
identical header and is still detected as a cycle.
@linkdata
linkdata marked this pull request as ready for review July 22, 2026 19:56
@linkdata
linkdata merged commit 6fd13a2 into main Jul 22, 2026
7 checks passed
@linkdata
linkdata deleted the fix/tagexpand-aliased-slice-views-203 branch July 22, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TagExpand drops tags from aliased slice views

1 participant