Skip to content

[RemoteMirror] Correctly project enums with zero-sized payloads #67257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 12, 2023
Merged

[RemoteMirror] Correctly project enums with zero-sized payloads #67257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions stdlib/public/RemoteInspection/TypeLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,13 +653,16 @@ class SinglePayloadEnumTypeInfo: public EnumTypeInfo {
// }
// ```
class TaggedMultiPayloadEnumTypeInfo: public EnumTypeInfo {
unsigned NumEffectivePayloadCases;
public:
TaggedMultiPayloadEnumTypeInfo(unsigned Size, unsigned Alignment,
unsigned Stride, unsigned NumExtraInhabitants,
bool BitwiseTakable,
const std::vector<FieldInfo> &Cases)
const std::vector<FieldInfo> &Cases,
unsigned NumEffectivePayloadCases)
: EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, EnumKind::MultiPayloadEnum, Cases) {
BitwiseTakable, EnumKind::MultiPayloadEnum, Cases),
NumEffectivePayloadCases(NumEffectivePayloadCases) {
// Definition of "multi-payload enum"
assert(getCases().size() > 1); // At least 2 cases
assert(Cases[0].TR != 0); // At least 2 payloads
Expand Down Expand Up @@ -698,7 +701,7 @@ class TaggedMultiPayloadEnumTypeInfo: public EnumTypeInfo {
remote::RemoteAddress address,
int *CaseIndex) const override {
unsigned long PayloadSize = getPayloadSize();
unsigned PayloadCount = getNumPayloadCases();
unsigned PayloadCount = NumEffectivePayloadCases;
unsigned NumCases = getNumCases();
unsigned TagSize = getSize() - PayloadSize;
unsigned tag = 0;
Expand Down Expand Up @@ -1030,15 +1033,24 @@ class BitMask {
// bits in the payload.
class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
BitMask spareBitsMask;
// "Effective" payload cases includes those with
// generic payload and non-generic cases that are
// statically known to have non-zero size.
// It does not include cases with payloads that are
// non-generic and zero-sized (these are treated as
// non-payload cases for many purposes).
unsigned NumEffectivePayloadCases;
public:
MultiPayloadEnumTypeInfo(unsigned Size, unsigned Alignment,
unsigned Stride, unsigned NumExtraInhabitants,
bool BitwiseTakable,
const std::vector<FieldInfo> &Cases,
BitMask spareBitsMask)
BitMask spareBitsMask,
unsigned NumEffectivePayloadCases)
: EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, EnumKind::MultiPayloadEnum, Cases),
spareBitsMask(spareBitsMask) {
spareBitsMask(spareBitsMask),
NumEffectivePayloadCases(NumEffectivePayloadCases) {
assert(Cases[0].TR != 0);
assert(Cases[1].TR != 0);
assert(getNumNonEmptyPayloadCases() > 1);
Expand Down Expand Up @@ -1125,7 +1137,6 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
remote::RemoteAddress address,
int *CaseIndex) const override {
unsigned long payloadSize = getPayloadSize();
unsigned NumPayloadCases = getNumPayloadCases();

// Extra Tag (if any) holds upper bits of case value
auto extraTagSize = getSize() - payloadSize;
Expand Down Expand Up @@ -1156,7 +1167,7 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
}

// If the above identifies a payload case, we're done
if (static_cast<unsigned>(tagValue) < NumPayloadCases) {
if (static_cast<unsigned>(tagValue) < NumEffectivePayloadCases) {
*CaseIndex = tagValue;
return true;
}
Expand All @@ -1173,9 +1184,9 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {

int ComputedCase = 0;
if (occupiedBitCount >= 32) {
ComputedCase = payloadValue + NumPayloadCases;
ComputedCase = payloadValue + NumEffectivePayloadCases;
} else {
ComputedCase = (((tagValue - NumPayloadCases) << occupiedBitCount) | payloadValue) + NumPayloadCases;
ComputedCase = (((tagValue - NumEffectivePayloadCases) << occupiedBitCount) | payloadValue) + NumEffectivePayloadCases;
}

if (static_cast<unsigned>(ComputedCase) < getNumCases()) {
Expand All @@ -1193,8 +1204,8 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
// * The remainder of the payload bits (for non-payload cases)
// This computes the bits used for the payload tag.
BitMask getMultiPayloadTagBitsMask() const {
auto payloadTagValues = getNumPayloadCases() - 1;
if (getNumCases() > getNumPayloadCases()) {
auto payloadTagValues = NumEffectivePayloadCases - 1;
if (getNumCases() > NumEffectivePayloadCases) {
payloadTagValues += 1;
}
int payloadTagBits = 0;
Expand Down Expand Up @@ -2257,7 +2268,7 @@ class EnumTypeInfoBuilder {
Stride = 1;
return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, Cases);
BitwiseTakable, Cases, EffectivePayloadCases);
}

// This is a multi-payload enum that:
Expand Down Expand Up @@ -2291,7 +2302,7 @@ class EnumTypeInfoBuilder {
// If there are no spare bits, use the "simple" tag-only implementation.
return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, Cases);
BitwiseTakable, Cases, EffectivePayloadCases);
}

#if 0 // TODO: This should be !defined(NDEBUG)
Expand All @@ -2314,7 +2325,8 @@ class EnumTypeInfoBuilder {
// Use compiler-provided spare bit information
return TC.makeTypeInfo<MultiPayloadEnumTypeInfo>(
Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, Cases, spareBitsMask);
BitwiseTakable, Cases, spareBitsMask,
EffectivePayloadCases);
}

// Either there was no compiler data or it didn't make sense
Expand All @@ -2339,13 +2351,14 @@ class EnumTypeInfoBuilder {
// above only returns an empty mask when the mask is really empty,
return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, Cases);
BitwiseTakable, Cases, EffectivePayloadCases);
} else {
// General case can mix spare bits and extra discriminator
// It obviously relies on having an accurate spare bit mask.
return TC.makeTypeInfo<MultiPayloadEnumTypeInfo>(
Size, Alignment, Stride, NumExtraInhabitants,
BitwiseTakable, Cases, spareBitsMask);
BitwiseTakable, Cases, spareBitsMask,
EffectivePayloadCases);
}
}
};
Expand Down
72 changes: 72 additions & 0 deletions validation-test/Reflection/reflect_Enum_values2.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values2
// RUN: %target-codesign %t/reflect_Enum_values2

// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values2 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants

// REQUIRES: objc_interop
// REQUIRES: executable_test
// UNSUPPORTED: use_os_stdlib

import SwiftReflectionTest

enum EnumB {
case a
case b(Int8)
case c(Int8)
case d(Void)
case e(Void)
case RIGHT(Int8)
case f(Int8)
case g(Void)
case LEFT(Int8, Int64)
}

class PlaceSummaryUnit {
let t: EnumB
init(t: EnumB) { self.t = t }
}

reflect(object: PlaceSummaryUnit(t: .RIGHT(5)))

// CHECKALL: Reflecting an object.
// CHECKALL-NEXT: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (class reflect_Enum_values2.PlaceSummaryUnit)

// CHECKALL: Type info

// TODO: Check the type layout for 64- and 32-bit targets

reflect(enumValue: PlaceSummaryUnit(t: .RIGHT(5)).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
// CHECKALL-NEXT: Value: .RIGHT(_)

reflect(enumValue: PlaceSummaryUnit(t: .a).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
// CHECKALL-NEXT: Value: .a

reflect(enumValue: PlaceSummaryUnit(t: .LEFT(1,2)).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
// CHECKALL-NEXT: Value: .LEFT(_)

reflect(enumValue: PlaceSummaryUnit(t: .d(())).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
// CHECKALL-NEXT: Value: .d(_)

doneReflecting()

// CHECKALL: Done.

85 changes: 85 additions & 0 deletions validation-test/Reflection/reflect_Enum_values3.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values3
// RUN: %target-codesign %t/reflect_Enum_values3

// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values3 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants

// REQUIRES: objc_interop
// REQUIRES: executable_test
// UNSUPPORTED: use_os_stdlib

import SwiftReflectionTest

enum EnumB<T> {
case a
case b(Int8)
case c(T)
case d(Void)
case e(Void)
case RIGHT(Int8)
case f(Int8)
case g(Void)
case LEFT(Int8, Int64)
}

class PlaceSummaryUnit<T> {
let t: EnumB<T>
init(t: EnumB<T>) { self.t = t }
}

reflect(object: PlaceSummaryUnit<Int8>(t: .RIGHT(5)))

// CHECKALL: Reflecting an object.
// CHECKALL-NEXT: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_class reflect_Enum_values3.PlaceSummaryUnit
// CHECKALL-NEXT: (struct Swift.Int8))

// CHECKALL: Type info

// TODO: Check the type layout for 64- and 32-bit targets

reflect(enumValue: PlaceSummaryUnit<Int8>(t: .RIGHT(5)).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
// CHECKALL-NEXT: (struct Swift.Int8))
// CHECKALL-NEXT: Value: .RIGHT(_)

reflect(enumValue: PlaceSummaryUnit<Int8>(t: .a).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
// CHECKALL-NEXT: (struct Swift.Int8))
// CHECKALL-NEXT: Value: .a

reflect(enumValue: PlaceSummaryUnit<Int8>(t: .c(7)).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
// CHECKALL-NEXT: (struct Swift.Int8))
// CHECKALL-NEXT: Value: .c(_)

reflect(enumValue: PlaceSummaryUnit<Int8>(t: .LEFT(1,2)).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
// CHECKALL-NEXT: (struct Swift.Int8))
// CHECKALL-NEXT: Value: .LEFT(_)

reflect(enumValue: PlaceSummaryUnit<Int8>(t: .d(())).t)

// CHECKALL: Reflecting an enum value.
// CHECKALL-NEXT: Type reference:
// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
// CHECKALL-NEXT: (struct Swift.Int8))
// CHECKALL-NEXT: Value: .d(_)

doneReflecting()

// CHECKALL: Done.