Skip to content

Harden generic-enum fixtures against layout and refcount bugs#10

Merged
Kyle-Ye merged 3 commits into
mainfrom
feature/fix-output-tests
Apr 15, 2026
Merged

Harden generic-enum fixtures against layout and refcount bugs#10
Kyle-Ye merged 3 commits into
mainfrom
feature/fix-output-tests

Conversation

@Kyle-Ye

@Kyle-Ye Kyle-Ye commented Apr 15, 2026

Copy link
Copy Markdown
Member

Summary

GestureOutputCompatibilityTests and GesturePhaseCompatibilityTests each
build their cases via a make(tag:_:) helper that (a) writes case fields at
hand-computed offsets and (b) extracts the value with load(as:) + defer deallocate. Two latent bugs made iOS Simulator discovery crash at xctest
bootstrap:

  1. Offset bug. .empty(GestureOutputEmptyReason, GestureOutputMetadata?)
    wrote metadata at stride(GestureOutputEmptyReason) == 1, but the case's
    tuple layout aligns GestureOutputMetadata? (alignment 8) to offset 8.
    The metadata bytes landed in the payload's padding region; reading the
    value back tripped initializeWithCopy during Swift Testing's test
    discovery (xctest at initializeWithCopy for GestureOutput /
    xctest at static GestureOutput.make(tag:_:)). The same failure mode is
    latent in GesturePhase.blocked(value:blockedBy:) for any Value whose
    stride does not match GestureNodeID's alignment — it only worked
    because the tests used Value = Int.

  2. Refcount leak. load(as:) bumps the returned value's retain count
    while deallocate() skips destroying the memory — whenever the payload
    carried a bridgeObject field, the retain initializeMemory wrote in
    place was leaked. The test values used so far are POD, so it was
    invisible until someone tested a String-carrying Value.

Both fixtures now share a tuple-based make<Payload>(tag:payload:):

  • The payload is typed as a Swift tuple ((reason, metadata),
    (value, blockedBy), () for no-payload cases) so the compiler computes
    the correct field offsets and alignment padding for every case.
  • Allocation uses UnsafeMutablePointer<…>.allocate(capacity:) + move(),
    transferring ownership out of the scratch slot instead of the leaky
    load+deallocate dance.

Also adds .active/.blocked tests for GesturePhase<String> and
.value/.finalValue tests for GestureOutput<String> to exercise a
bridgeObject-carrying payload through initializeWithCopy — these would
trip the leak under the old pattern.

Test plan

  • xcodebuild test -only-testing:OpenGesturesTests on iPhone 17 Pro / iOS 26.2 Simulator
  • OPENGESTURES_COMPATIBILITY_TEST=0 xcodebuild test -only-testing:OpenGesturesCompatibilityTests on the same simulator
  • OPENGESTURES_COMPATIBILITY_TEST=1 OPENGESTURES_USE_LOCAL_DEPS=1 xcodebuild test -only-testing:OpenGesturesCompatibilityTests on the same simulator

Kyle-Ye added 2 commits April 16, 2026 02:33
The `.empty(reason, metadata:)` builder wrote metadata at
`MemoryLayout<GestureOutputEmptyReason>.stride == 1`, but the case's
tuple layout aligns `GestureOutputMetadata?` (alignment 8) to offset
8 — the metadata bytes were landing in the payload's padding region.
Reading the value back tripped `initializeWithCopy` during Swift
Testing's discovery on iOS Simulator, crashing xctest at bootstrap.

- Build the payload as a Swift tuple so the compiler computes the
  correct field offsets and refcount handling for every case.
- Replace `UnsafeMutableRawPointer.allocate` + `load(as:)` with
  `UnsafeMutablePointer<GestureOutput>.allocate(capacity:)` + `move()`
  to transfer ownership out of the scratch slot, avoiding the extra
  retain that the load+deallocate pattern leaked.
- Promote the tuple-based enum builder into a shared `makeEnum<T, Payload>`
  in `Metadata+Enum.swift`; drop the per-file `make(tag:_:)` helpers.
- Apply the same pattern to `GesturePhase`'s fixture (previously worked only
  for `Value = Int` by coincidence — `.blocked(value:blockedBy:)` uses
  `stride(Value)` as the second field's offset).
- Add `.active`/`.blocked` tests for `GesturePhase<String>` and
  `.value`/`.finalValue` tests for `GestureOutput<String>` so a
  bridgeObject-carrying payload round-trips through `initializeWithCopy`.
@Kyle-Ye Kyle-Ye changed the title Fix GestureOutput test fixture enum-layout bugs Harden generic-enum fixtures against layout and refcount bugs Apr 15, 2026
@Kyle-Ye Kyle-Ye merged commit d9f1a44 into main Apr 15, 2026
4 checks passed
@Kyle-Ye Kyle-Ye deleted the feature/fix-output-tests branch April 15, 2026 18:55
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.

1 participant