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 088cef3..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) { @@ -1385,31 +1385,33 @@ 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_offset = item.offset; for (auto &&child : enum_case.children) { std::visit(children_emitter, child); } 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_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; @@ -1421,7 +1423,7 @@ void Builder::Emitter>::enter(const RangeIt _invalid = true; } } - _emitted_size = item.offset; + _emitted_offset = item.offset; } void Builder::Emitter>::finish() { 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