Speculatively revert "IRGen: Make case numbering consistent for enums with empty payloads"

I suspect its breaking the build. I must've screwed up my testing.

This reverts commit 86e6b81.

Author: slavapestov

lib/IRGen/GenDecl.cpp

@@ -2588,14 +2588,7 @@ StringRef IRGenModule::mangleType(CanType type, SmallVectorImpl<char> &buffer) {
   return StringRef(buffer.data(), buffer.size());
 }
 
-/// Do we have to use resilient access patterns when working with this
-/// declaration?
-///
-/// IRGen is primarily concerned with resilient handling of the following:
-/// - For structs, a struct's size might change
-/// - For enums, new cases can be added
-/// - For classes, the superclass might change the size or number
-///   of stored properties
+/// Is the given declaration resilient?
 bool IRGenModule::isResilient(Decl *D, ResilienceScope scope) {
   auto NTD = dyn_cast<NominalTypeDecl>(D);
   if (!NTD)
@@ -2611,23 +2604,6 @@ bool IRGenModule::isResilient(Decl *D, ResilienceScope scope) {
   llvm_unreachable("Bad resilience scope");
 }
 
-// The most general resilience scope where the given declaration is visible.
-ResilienceScope IRGenModule::getResilienceScopeForAccess(NominalTypeDecl *decl) {
-  if (decl->getModuleContext() == SILMod->getSwiftModule() &&
-      decl->getFormalAccess() != Accessibility::Public)
-    return ResilienceScope::Component;
-  return ResilienceScope::Universal;
-}
-
-// The most general resilience scope which has knowledge of the declaration's
-// layout. Calling isResilient() with this scope will always return false.
-ResilienceScope IRGenModule::getResilienceScopeForLayout(NominalTypeDecl *decl) {
-  if (isResilient(decl, ResilienceScope::Universal))
-    return ResilienceScope::Component;
-
-  return getResilienceScopeForAccess(decl);
-}
-
 /// Fetch the witness table access function for a protocol conformance.
 llvm::Function *
 IRGenModule::getAddrOfWitnessTableAccessFunction(

lib/IRGen/GenEnum.cpp

@@ -4598,22 +4598,17 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,
   std::vector<Element> elementsWithPayload;
   std::vector<Element> elementsWithNoPayload;
 
-  // Resilient enums are manipulated as opaque values, except we still
-  // make the following assumptions:
-  // 1) Physical case indices won't change
-  // 2) The indirect-ness of cases won't change
-  // 3) Payload types won't change in a non-resilient way
-  bool isResilient = TC.IGM.isResilient(theEnum, ResilienceScope::Component);
-  
-  // The most general resilience scope where the enum type is visible.
-  // Case numbering must not depend on any information that is not static
-  // in this resilience scope.
-  ResilienceScope accessScope = TC.IGM.getResilienceScopeForAccess(theEnum);
-
-  // The most general resilience scope where the enum's layout is known.
-  // Fixed-size optimizations can be applied if all payload types are
-  // fixed-size from this resilience scope.
-  ResilienceScope layoutScope = TC.IGM.getResilienceScopeForLayout(theEnum);
+  // The most general resilience scope that can have knowledge of this
+  // enum's layout. If all payload types have a fixed size in this
+  // resilience scope, we can make further assumptions to optimize
+  // layout.
+  ResilienceScope scope = ResilienceScope::Universal;
+
+  // TODO: Replace this with 'public or internal with @availability' check
+  // once that is in place
+  if (theEnum->getFormalAccess() != Accessibility::Public ||
+      TC.IGM.isResilient(theEnum, ResilienceScope::Universal))
+    scope = ResilienceScope::Component;
 
   for (auto elt : theEnum->getAllElements()) {
     numElements++;
@@ -4643,20 +4638,14 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,
       = TC.tryGetCompleteTypeInfo(origArgLoweredTy.getSwiftRValueType());
     assert(origArgTI && "didn't complete type info?!");
 
-    // If the unsubstituted argument contains a generic parameter type, or
-    // is not fixed-size in all resilience domains that have knowledge of
-    // this enum's layout, we need to constrain our layout optimizations to
-    // what the runtime can reproduce.
-    if (!isResilient &&
-        !origArgTI->isFixedSize(layoutScope))
+    // If the unsubstituted argument contains a generic parameter type, or if
+    // the substituted argument is not universally fixed-size, we need to
+    // constrain our layout optimizations to what the runtime can reproduce.
+    if (!origArgTI->isFixedSize(scope))
       alwaysFixedSize = IsNotFixedSize;
 
-    // If the payload is empty, turn the case into a no-payload case, but
-    // only if case numbering remains unchanged from all resilience domains
-    // that can see the enum.
-    if (origArgTI->isFixedSize(accessScope) &&
-        isa<LoadableTypeInfo>(origArgTI) &&
-        cast<LoadableTypeInfo>(origArgTI)->isKnownEmpty()) {
+    auto loadableOrigArgTI = dyn_cast<LoadableTypeInfo>(origArgTI);
+    if (loadableOrigArgTI && loadableOrigArgTI->isKnownEmpty()) {
       elementsWithNoPayload.push_back({elt, nullptr, nullptr});
     } else {
       // *Now* apply the substitutions and get the type info for the instance's
@@ -4666,20 +4655,16 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,
       auto *substArgTI = &TC.IGM.getTypeInfo(fieldTy);
 
       elementsWithPayload.push_back({elt, substArgTI, origArgTI});
+      if (!substArgTI->isFixedSize())
+        tik = Opaque;
+      else if (!substArgTI->isLoadable() && tik > Fixed)
+        tik = Fixed;
 
-      if (!isResilient) {
-        if (!substArgTI->isFixedSize(ResilienceScope::Component))
-          tik = Opaque;
-        else if (!substArgTI->isLoadable() && tik > Fixed)
-          tik = Fixed;
-
-        // If the substituted argument contains a type that is not fixed-size
-        // in all resilience domains that have knowledge of this enum's layout,
-        // we need to constrain our layout optimizations to what the runtime
-        // can reproduce.
-        if (!substArgTI->isFixedSize(layoutScope))
-          alwaysFixedSize = IsNotFixedSize;
-      }
+      // If the substituted argument contains a type that is not universally
+      // fixed-size, we need to constrain our layout optimizations to what
+      // the runtime can reproduce.
+      if (!substArgTI->isFixedSize(scope))
+        alwaysFixedSize = IsNotFixedSize;
     }
   }
 
@@ -4688,7 +4673,12 @@ EnumImplStrategy *EnumImplStrategy::get(TypeConverter &TC,
            + elementsWithNoPayload.size()
          && "not all elements accounted for");
 
-  if (isResilient) {
+  // Resilient enums are manipulated as opaque values, except we still
+  // make the following assumptions:
+  // 1) Physical case indices won't change
+  // 2) The indirect-ness of cases won't change
+  // 3) Payload types won't change in a non-resilient way
+  if (TC.IGM.isResilient(theEnum, ResilienceScope::Component)) {
     return new ResilientEnumImplStrategy(TC.IGM,
                                          numElements,
                                          std::move(elementsWithPayload),

lib/IRGen/IRGenModule.h

@@ -504,8 +504,6 @@ class IRGenModule {
   }
 
   bool isResilient(Decl *decl, ResilienceScope scope);
-  ResilienceScope getResilienceScopeForAccess(NominalTypeDecl *decl);
-  ResilienceScope getResilienceScopeForLayout(NominalTypeDecl *decl);
 
   SpareBitVector getSpareBitsForType(llvm::Type *scalarTy, Size size);
 

test/Inputs/resilient_enum.swift

@@ -40,6 +40,8 @@ public struct Color {
   case Bespoke(Color, Color)
 }
 
+// Fixed-layout enum with resilient members
+
 // Resilient enum
 public enum Medium {
   // Empty cases
@@ -58,21 +60,6 @@ public indirect enum IndirectApproach {
   case Angle(Double)
 }
 
-// Resilient enum with resilient empty payload case
-public struct EmptyStruct {
-  public init() {}
-}
-
-public enum ResilientEnumWithEmptyCase {
-  case A // should always be case 1
-  case B // should always be case 2
-  case Empty(EmptyStruct) // should always be case 0
-}
-
-public func getResilientEnumWithEmptyCase() -> [ResilientEnumWithEmptyCase] {
-  return [.A, .B, .Empty(EmptyStruct())]
-}
-
 // Specific enum implementations for executable tests
 public enum ResilientEmptyEnum {
   case X

test/Interpreter/enum_resilience.swift

@@ -379,25 +379,4 @@ ResilientEnumTestSuite.test("DynamicLayoutMultiPayload2") {
   expectEqual(b, [0, 1, 2, 3])
 }
 
-// Make sure case numbers round-trip if payload has zero size
-
-ResilientEnumTestSuite.test("ResilientEnumWithEmptyCase") {
-  let a: [ResilientEnumWithEmptyCase] = getResilientEnumWithEmptyCase()
-
-  let b: [Int] = a.map {
-    switch $0 {
-    case .A:
-      return 0
-    case .B:
-      return 1
-    case .Empty:
-      return 2
-    default:
-      return -1
-    }
-  }
-
-  expectEqual(b, [0, 1, 2])
-}
-
 runAllTests()