Fix reuse of the payload area when the discriminator != 0

include/swift/Reflection/ReflectionContext.h

@@ -751,44 +751,51 @@ class ReflectionContext
         return false;
       unsigned long NonPayloadCaseCount = FieldCount - 1;
       unsigned long PayloadExtraInhabitants = PayloadCase.TI.getNumExtraInhabitants();
-      unsigned discriminator = 0;
+      unsigned Discriminator = 0;
       auto PayloadSize = PayloadCase.TI.getSize();
       if (NonPayloadCaseCount >= PayloadExtraInhabitants) {
         // There are more cases than inhabitants, we need a separate discriminator.
         auto TagInfo = getEnumTagCounts(PayloadSize, NonPayloadCaseCount, 1);
         auto TagSize = TagInfo.numTagBytes;
         auto TagAddress = RemoteAddress(EnumAddress.getAddressData() + PayloadSize);
-        if (!getReader().readInteger(TagAddress, TagSize, &discriminator))
+        if (!getReader().readInteger(TagAddress, TagSize, &Discriminator)) {
+          printf(">>>> readXI failed to read discriminator\n\n");
           return false;
+        }
       }
 
-      if (PayloadExtraInhabitants == 0) {
-        // Payload has no XI, so discriminator fully determines the case
-        *CaseIndex = discriminator;
+      if (PayloadSize == 0) {
+        // Payload carries no information, so discriminator fully determines the case
+        *CaseIndex = Discriminator;
         return true;
-      } else if (discriminator == 0) {
-        // The value overlays the payload ... ask the payload to decode it.
-        int t;
-        if (!PayloadCase.TI.readExtraInhabitantIndex(getReader(), EnumAddress, &t)) {
+      } else if (Discriminator == 0) {
+        // The payload area carries all the information...
+        if (PayloadExtraInhabitants == 0) {
+          *CaseIndex = 0;
+          return true;
+        }
+        int XITag = 0;
+        if (!PayloadCase.TI.readExtraInhabitantIndex(getReader(), EnumAddress, &XITag)) {
           return false;
         }
-        if (t < 0) {
-          *CaseIndex = 0;
+        if (XITag < 0) { // Valid (not extra) inhabitant
+          *CaseIndex = 0; // Payload case is always #0
           return true;
-        } else if ((unsigned long)t <= NonPayloadCaseCount) {
-          *CaseIndex = t + 1;
+        } else if ((unsigned)XITag <= NonPayloadCaseCount) {
+          *CaseIndex = XITag + 1;
           return true;
         }
         return false;
       } else {
-        // The entire payload area is available for additional cases:
-        auto TagSize = std::max(PayloadSize, 4U); // XXX TODO XXX CHECK THIS
-        auto offset = 1 + PayloadExtraInhabitants; // Cases coded with discriminator = 0
-        unsigned casesInPayload = 1 << (TagSize * 8U);
-        unsigned payloadCode;
-        if (!getReader().readInteger(EnumAddress, TagSize, &payloadCode))
+        // No payload: Payload area is reused for more cases
+        unsigned PayloadTag = 0;
+        auto PayloadTagSize = std::min(PayloadSize, decltype(PayloadSize)(sizeof(PayloadTag)));
+        if (!getReader().readInteger(EnumAddress, PayloadTagSize, &PayloadTag)) {
           return false;
-        *CaseIndex = offset + (discriminator - 1) * casesInPayload + payloadCode;
+        }
+        auto XICases = 1U + PayloadExtraInhabitants; // Cases coded with XIs when discriminator = 0
+        auto PayloadCases = 1U << (PayloadTagSize * 8U);
+        *CaseIndex = XICases + (Discriminator - 1) * PayloadCases + PayloadTag;
         return true;
       }
     }

stdlib/public/Reflection/TypeLowering.cpp

@@ -258,11 +258,24 @@ bool RecordTypeInfo::readExtraInhabitantIndex(remote::MemoryReader &reader,
                                               int *extraInhabitantIndex) const {
   switch (SubKind) {
   case RecordKind::Invalid:
-  case RecordKind::ThickFunction:
   case RecordKind::OpaqueExistential:
   case RecordKind::ClosureContext:
     return false;
 
+  case RecordKind::ThickFunction: {
+    if (Fields.size() != 2) {
+      return false;
+    }
+    auto function = Fields[0];
+    auto context = Fields[1];
+    if (function.Offset != 0) {
+      return false;
+    }
+    auto functionFieldAddress = address;
+    return function.TI.readExtraInhabitantIndex(
+      reader, functionFieldAddress, extraInhabitantIndex);
+  }
+
   case RecordKind::ClassExistential:
   case RecordKind::ExistentialMetatype:
   case RecordKind::ErrorExistential:

validation-test/Reflection/reflect_Enum_value.swift

@@ -61,8 +61,9 @@ var a: Int
 
 enum OneStructPayload {
 case payloadA(StructInt)
-case otherA
-case otherB
+case cowboyAlice
+case cowboyBob
+case cowboyCharlie
 }
 
 reflect(enumValue: OneStructPayload.payloadA(StructInt(a: 0)))
@@ -72,12 +73,12 @@ reflect(enumValue: OneStructPayload.payloadA(StructInt(a: 0)))
 // CHECK-NEXT: (enum reflect_Enum_value.OneStructPayload)
 // CHECK-NEXT: Value: .payloadA(_)
 
-reflect(enumValue: OneStructPayload.otherA)
+reflect(enumValue: OneStructPayload.cowboyCharlie)
 
 // CHECK: Reflecting an enum value.
 // CHECK-NEXT: Type reference:
 // CHECK-NEXT: (enum reflect_Enum_value.OneStructPayload)
-// CHECK-NEXT: Value: .otherA
+// CHECK-NEXT: Value: .cowboyCharlie
 
 @objc class ObjCClass : NSObject {
 var a: Int = 0
@@ -173,14 +174,14 @@ reflect(enumValue: Optional<Optional<OneSwiftClassPayload>>.some(.none))
 // CHECK-NEXT:     (enum reflect_Enum_value.OneSwiftClassPayload)))
 // CHECK-NEXT: Value: .some(.none)
 
-reflect(enumValue: Optional<Optional<OneSwiftClassPayload>>.some(.some(.otherA)))
+reflect(enumValue: Optional<Optional<OneSwiftClassPayload>>.some(.some(.otherC)))
 
 // CHECK: Reflecting an enum value.
 // CHECK-NEXT: Type reference:
 // CHECK-NEXT: (bound_generic_enum Swift.Optional
 // CHECK-NEXT:   (bound_generic_enum Swift.Optional
 // CHECK-NEXT:     (enum reflect_Enum_value.OneSwiftClassPayload)))
-// CHECK-NEXT: Value: .some(.some(.otherA))
+// CHECK-NEXT: Value: .some(.some(.otherC))
 
 reflect(enumValue: Optional<Optional<OneSwiftClassPayload>>.some(.some(.otherE)))
 
@@ -211,12 +212,12 @@ case otherB
 case payloadA(MixedStruct)
 }
 
-reflect(enumValue: OneMixedStructPayload.otherA)
+reflect(enumValue: OneMixedStructPayload.otherB)
 
 // CHECK: Reflecting an enum value.
 // CHECK-NEXT: Type reference:
 // CHECK-NEXT: (enum reflect_Enum_value.OneMixedStructPayload)
-// CHECK-NEXT: Value: .otherA
+// CHECK-NEXT: Value: .otherB
 
 reflect(enumValue: OneMixedStructPayload.payloadA(MixedStruct()))
 
@@ -247,7 +248,138 @@ reflect(enumValue: OneNestedPayload.alternateB)
 // CHECK-NEXT: (enum reflect_Enum_value.OneNestedPayload)
 // CHECK-NEXT: Value: .alternateB
 
-// XXX TODO: enum with tuple payload, enum with optional payload, indirect enum, enum with closure/function payload XXX
+
+enum OneTuplePayload {
+case holderA((i: Int, c: SwiftClass))
+case emptyA
+case emptyB
+case emptyC
+}
+
+reflect(enumValue: OneTuplePayload.holderA((i: 7, c: SwiftClass())))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneTuplePayload)
+// CHECK-NEXT: Value: .holderA(_)
+
+reflect(enumValue: OneTuplePayload.emptyB)
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneTuplePayload)
+// CHECK-NEXT: Value: .emptyB
+
+func foo() -> Int { return 7; }
+
+enum OneFunctionPayload {
+case cargoA(() -> Int)
+case alternateA
+case alternateB
+case alternateC
+}
+
+reflect(enumValue: OneFunctionPayload.cargoA(foo))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneFunctionPayload)
+// CHECK-NEXT: Value: .cargoA(_)
+
+reflect(enumValue: OneFunctionPayload.alternateC)
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneFunctionPayload)
+// CHECK-NEXT: Value: .alternateC
+
+func tester1() {
+  let a = 7
+
+  func foo() -> Int { return a; }
+
+  enum OneClosurePayload {
+  case cargoA(() -> Int)
+  case alternateA
+  case alternateB
+  case alternateC
+  }
+
+  reflect(enumValue: OneClosurePayload.cargoA(foo))
+
+  // CHECK: Reflecting an enum value.
+  // CHECK-NEXT: Type reference:
+
+  // XXX TODO: Figure out why the type reference is dumped differently sometimes:
+  // XXXX-NEXT: (nominal with unmangled suffix
+  // XXXX-NEXT: (enum OneClosurePayload #1 in reflect_Enum_value.tester1() -> ())
+
+  // CHECK: Value: .cargoA(_)
+
+  reflect(enumValue: OneClosurePayload.alternateB)
+
+  // CHECK: Reflecting an enum value.
+  // CHECK-NEXT: Type reference:
+
+  // XXX TODO: Figure out why the type reference is dumped differently sometimes:
+  // XXXX-NEXT: (nominal with unmangled suffix
+  // XXXX-NEXT: (enum OneClosurePayload #1 in reflect_Enum_value.tester1() -> ())
+
+  // CHECK: Value: .alternateB
+}
+
+tester1()
+
+
+enum OneOptionalPayload {
+case boxA(Optional<Int>)
+case unboxA
+case unboxB
+case unboxC
+case unboxD
+case unboxE
+}
+
+reflect(enumValue: OneOptionalPayload.boxA(7))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneOptionalPayload)
+// CHECK-NEXT: Value: .boxA(.some(_))
+
+reflect(enumValue: OneOptionalPayload.unboxE)
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneOptionalPayload)
+// CHECK-NEXT: Value: .unboxE
+
+indirect enum OneIndirectPayload {
+case child(OneIndirectPayload)
+case leafA
+case leafB
+case leafC
+case leafD
+case leafE
+case leafF
+}
+
+reflect(enumValue: OneIndirectPayload.child(.leafF))
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneIndirectPayload)
+// CHECK-NEXT: Value: .child(_)
+
+reflect(enumValue: OneIndirectPayload.leafF)
+
+// CHECK: Reflecting an enum value.
+// CHECK-NEXT: Type reference:
+// CHECK-NEXT: (enum reflect_Enum_value.OneIndirectPayload)
+// CHECK-NEXT: Value: .leafF
+
+
+// XXX TODO: test enum with thin function payload XXX
 
 doneReflecting()
 // CHECK: Done.