From 57e68bb883d8fcb01c69fdcf9908963cf6dd3cb1 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Thu, 22 Oct 2026 18:43:04 +0200 Subject: [PATCH 1/4] Start enum case emitters at the enum's offset Case items share the enum item's offset space, and the comparison starts each case at the enum's offset. Starting the case emitter at 0 encoded an extra skip equal to the enum's offset, so enums laid out inline at a nonzero offset compared the wrong bytes. Co-Authored-By: Claude Opus 5.5 --- Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index 088cef3..a2f3d93 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -1385,7 +1385,10 @@ void Builder::Emitter>::operator()(const En emit_value((void *)item.type); } + // Case items have offsets in the same space as the enum item, and the + // comparison starts each case at the enum's offset. auto children_emitter = Emitter>(_data); + children_emitter._emitted_size = item.offset; for (auto &&child : enum_case.children) { std::visit(children_emitter, child); } From c2386c77658470d9e45b0866a644e23b1a1adabe Mon Sep 17 00:00:00 2001 From: James Moschou Date: Thu, 22 Oct 2026 18:56:33 +0200 Subject: [PATCH 2/4] Propagate invalid enum case layouts to the enclosing enum Match AttributeGraph: an enum's layout is invalid if any case's children are invalid, not only when a case extends past the enum's size. Co-Authored-By: Claude Opus 5.5 --- Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index a2f3d93..2d9b5a3 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -1395,9 +1395,8 @@ void Builder::Emitter>::operator()(const En is_first = false; - if (!_invalid && !children_emitter.is_invalid()) { - _invalid = _emitted_size + item.type->vw_size() < children_emitter.emitted_size(); - } + _invalid = _invalid || children_emitter.is_invalid() || + _emitted_size + item.type->vw_size() < children_emitter.emitted_size(); } } From f0844263e7d6da7d8da920e4b924acbf03cf7c6d Mon Sep 17 00:00:00 2001 From: James Moschou Date: Thu, 22 Oct 2026 19:03:52 +0200 Subject: [PATCH 3/4] Rename Emitter::_emitted_size to _emitted_offset The field is the offset in the value up to which layout entries have been emitted, not a size. Co-Authored-By: Claude Opus 5.5 --- Sources/ComputeCxx/Comparison/Builder.h | 10 ++++--- .../Comparison/LayoutDescriptor.cpp | 26 +++++++++---------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/Sources/ComputeCxx/Comparison/Builder.h b/Sources/ComputeCxx/Comparison/Builder.h index bd559ca..0e601de 100644 --- a/Sources/ComputeCxx/Comparison/Builder.h +++ b/Sources/ComputeCxx/Comparison/Builder.h @@ -56,7 +56,8 @@ class Builder : public swift::metadata_visitor { class Emitter { private: T *_Nonnull _data; - size_t _emitted_size = 0; + // Offset in the value up to which layout entries have been emitted. + size_t _emitted_offset = 0; bool _invalid = false; public: @@ -70,7 +71,7 @@ class Builder : public swift::metadata_visitor { void enter(const RangeItem &range_item); const vector &data() const { return *_data; }; - size_t emitted_size() const { return _emitted_size; }; + size_t emitted_offset() const { return _emitted_offset; }; bool is_invalid() const { return _invalid; }; void set_invalid(bool invalid) { _invalid = invalid; }; @@ -81,7 +82,8 @@ class Builder : public swift::metadata_visitor { class Emitter> { private: vector *_Nonnull _data; - size_t _emitted_size = 0; + // Offset in the value up to which layout entries have been emitted. + size_t _emitted_offset = 0; bool _invalid = false; public: @@ -97,7 +99,7 @@ class Builder : public swift::metadata_visitor { void enter(const RangeItem &range_item); const vector &data() const { return *_data; }; - size_t emitted_size() const { return _emitted_size; }; + size_t emitted_offset() const { return _emitted_offset; }; bool is_invalid() const { return _invalid; }; void set_invalid(bool invalid) { _invalid = invalid; }; diff --git a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp index 2d9b5a3..281c782 100644 --- a/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp +++ b/Sources/ComputeCxx/Comparison/LayoutDescriptor.cpp @@ -969,7 +969,7 @@ ValueLayout Builder::commit(const swift::metadata &type) { return nullptr; } if (_heap_mode == HeapMode(0)) { - emitter.set_invalid(type.vw_size() < emitter.emitted_size()); + emitter.set_invalid(type.vw_size() < emitter.emitted_offset()); if (emitter.is_invalid()) { return ValueLayoutTrivial; } @@ -1268,7 +1268,7 @@ void Builder::Emitter>::operator()(const Da if (item_size != 0) { _data->push_back((item_size - 1) | 0x80); } - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const EqualsItem &item) { @@ -1276,7 +1276,7 @@ void Builder::Emitter>::operator()(const Eq _data->push_back((unsigned char)ValueLayoutEntryKind::Equals); emit_value((void *)item.type); emit_value((void *)item.equatable); - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const IndirectItem &item) { @@ -1284,21 +1284,21 @@ void Builder::Emitter>::operator()(const In _data->push_back((unsigned char)ValueLayoutEntryKind::Indirect); emit_value((void *)item.type); emit_value(nullptr); - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const ExistentialItem &item) { enter(item); _data->push_back((unsigned char)ValueLayoutEntryKind::Existential); emit_value((void *)item.type); - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const HeapRefItem &item) { enter(item); _data->push_back(item.is_capture_ref ? (unsigned char)ValueLayoutEntryKind::CaptureRef : (unsigned char)ValueLayoutEntryKind::HeapRef); - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const NestedItem &item) { @@ -1338,7 +1338,7 @@ void Builder::Emitter>::operator()(const Ne } } - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::operator()(const EnumItem &item) { @@ -1388,7 +1388,7 @@ void Builder::Emitter>::operator()(const En // Case items have offsets in the same space as the enum item, and the // comparison starts each case at the enum's offset. auto children_emitter = Emitter>(_data); - children_emitter._emitted_size = item.offset; + children_emitter._emitted_offset = item.offset; for (auto &&child : enum_case.children) { std::visit(children_emitter, child); } @@ -1396,22 +1396,22 @@ void Builder::Emitter>::operator()(const En is_first = false; _invalid = _invalid || children_emitter.is_invalid() || - _emitted_size + item.type->vw_size() < children_emitter.emitted_size(); + _emitted_offset + item.type->vw_size() < children_emitter.emitted_offset(); } } _data->push_back((unsigned char)ValueLayoutEntryKind::EnumEnd); - _emitted_size += item.size; + _emitted_offset += item.size; } void Builder::Emitter>::enter(const RangeItem &item) { if (!_invalid) { - if (item.offset >= _emitted_size) { + if (item.offset >= _emitted_offset) { // Emit number of bytes until item offset // encode as a series of numbers from 0-63, indicating chunks of // bytes 1-64 - size_t skip = item.offset - _emitted_size; + size_t skip = item.offset - _emitted_offset; while (skip > 0x40) { _data->push_back('\x7f'); skip -= 0x40; @@ -1423,7 +1423,7 @@ void Builder::Emitter>::enter(const RangeIt _invalid = true; } } - _emitted_size = item.offset; + _emitted_offset = item.offset; } void Builder::Emitter>::finish() { From 5c03cbab35f8637c74cb26eff49448aec75d7679 Mon Sep 17 00:00:00 2001 From: James Moschou Date: Thu, 22 Oct 2026 19:13:30 +0200 Subject: [PATCH 4/4] Add test for enum cases in a layout built while a nested layout is pending A synchronous layout build that finds a nested enum's layout still queued visits the enum inline at a nonzero offset. Check that values differing in an enum payload still compare unequal. Co-Authored-By: Claude Opus 5.5 --- .../Shared/PendingNestedLayoutTests.swift | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 Tests/ComputeLayoutDescriptorTests/Shared/PendingNestedLayoutTests.swift diff --git a/Tests/ComputeLayoutDescriptorTests/Shared/PendingNestedLayoutTests.swift b/Tests/ComputeLayoutDescriptorTests/Shared/PendingNestedLayoutTests.swift new file mode 100644 index 0000000..4991a06 --- /dev/null +++ b/Tests/ComputeLayoutDescriptorTests/Shared/PendingNestedLayoutTests.swift @@ -0,0 +1,69 @@ +import Foundation +import Testing + +// This suite relies on process isolation, which does not complete on Linux. +#if !os(Linux) + +private final class PendingBox {} + +private struct PendingPayload { + var name: String + var box: PendingBox +} + +/// The optional payload sits at offset 32. +private struct PendingInner { + var a: Int + var b: Int + var c: Int + var d: Int + var payload: PendingPayload? +} + +private struct PendingOuter { + var tag: Int + var inner: PendingInner +} + +@Suite(.serialized) +struct PendingNestedLayoutTests { + + /// A layout built synchronously while a nested enum's layout is still queued visits the enum's + /// cases inline, at a nonzero offset. The case contents must still be compared at their own offsets. + @Test + func enumCasesOfPendingNestedLayout() async { + await #expect(processExitsWith: .success) { + let mode = ComparisonOptions(mode: .equatableUnlessPOD) + + // Queue the optional's layout, leaving a placeholder in the layout cache. The background + // queue runs at low priority, so the placeholder outlives the synchronous fetch below. + _ = prefetchCompareValues(type: Metadata(PendingPayload?.self), options: mode, priority: 0) + + _ = prefetchCompareValues( + type: Metadata(PendingOuter.self), + options: ComparisonOptions( + rawValue: mode.rawValue | ComparisonOptions.fetchLayoutsSynchronously.rawValue + ), + priority: 0 + ) + + let box = PendingBox() + let lhs = PendingOuter( + tag: 0, + inner: PendingInner(a: 0, b: 0, c: 0, d: 0, payload: PendingPayload(name: "a", box: box)) + ) + let rhs = PendingOuter( + tag: 0, + inner: PendingInner(a: 0, b: 0, c: 0, d: 0, payload: PendingPayload(name: "b", box: box)) + ) + let equal = compareValues( + lhs, + rhs, + options: ComparisonOptions(rawValue: mode.rawValue | ComparisonOptions.copyOnWrite.rawValue) + ) + #expect(!equal, "expected values that differ in `payload.name` to compare unequal") + } + } +} + +#endif