Merge pull request #18449 from jrose-apple/4.2-default-on-your-futures

[4.2] [SIL] Don't drop a default when switching on a non-exhaustive enum

Author: Jacob Mizraji

include/swift/AST/Decl.h

@@ -3202,45 +3202,6 @@ class EnumDecl final : public NominalTypeDecl {
     return std::distance(eltRange.begin(), eltRange.end());
   }
 
-  /// If this is an enum with two cases, return the other case. Otherwise,
-  /// return nullptr.
-  EnumElementDecl *getOppositeBinaryDecl(EnumElementDecl *decl) const {
-    ElementRange range = getAllElements();
-    auto iter = range.begin();
-    if (iter == range.end())
-      return nullptr;
-    bool seenDecl = false;
-    EnumElementDecl *result = nullptr;
-    if (*iter == decl) {
-      seenDecl = true;
-    } else {
-      result = *iter;
-    }
-
-    ++iter;
-    if (iter == range.end())
-      return nullptr;
-    if (seenDecl) {
-      assert(!result);
-      result = *iter;
-    } else {
-      if (decl != *iter)
-        return nullptr;
-      seenDecl = true;
-    }
-    ++iter;
-
-    // If we reach this point, we saw the decl we were looking for and one other
-    // case. If we have any additional cases, then we do not have a binary enum.
-    if (iter != range.end())
-      return nullptr;
-
-    // This is always true since we have already returned earlier nullptr if we
-    // did not see the decl at all.
-    assert(seenDecl);
-    return result;
-  }
-
   /// If this enum has a unique element, return it. A unique element can
   /// either hold a value or not, and the existence of one unique element does
   /// not imply the existence or non-existence of the opposite unique element.
@@ -3327,7 +3288,23 @@ class EnumDecl final : public NominalTypeDecl {
   /// Note that this property is \e not necessarily true for all children of
   /// \p useDC. In particular, an inlinable function does not get to switch
   /// exhaustively over a non-exhaustive enum declared in the same module.
-  bool isExhaustive(const DeclContext *useDC) const;
+  ///
+  /// This is the predicate used when deciding if a switch statement needs a
+  /// default case. It should not be used for optimization or code generation.
+  ///
+  /// \sa isEffectivelyExhaustive
+  bool isFormallyExhaustive(const DeclContext *useDC) const;
+
+  /// True if the enum can be exhaustively switched within a function defined
+  /// within \p M, with \p expansion specifying whether the function is
+  /// inlinable.
+  ///
+  /// This is the predicate used when making optimization and code generation
+  /// decisions. It should not be used at the AST or semantic level.
+  ///
+  /// \sa isFormallyExhaustive
+  bool isEffectivelyExhaustive(ModuleDecl *M,
+                               ResilienceExpansion expansion) const;
 };
 
 /// StructDecl - This is the declaration of a struct, for example:

lib/AST/Decl.cpp

@@ -3189,7 +3189,7 @@ bool EnumDecl::hasOnlyCasesWithoutAssociatedValues() const {
   return true;
 }
 
-bool EnumDecl::isExhaustive(const DeclContext *useDC) const {
+bool EnumDecl::isFormallyExhaustive(const DeclContext *useDC) const {
   // Enums explicitly marked frozen are exhaustive.
   if (getAttrs().hasAttribute<FrozenAttr>())
     return true;
@@ -3236,6 +3236,22 @@ bool EnumDecl::isExhaustive(const DeclContext *useDC) const {
   return false;
 }
 
+bool EnumDecl::isEffectivelyExhaustive(ModuleDecl *M,
+                                       ResilienceExpansion expansion) const {
+  // Generated Swift code commits to handling garbage values of @objc enums,
+  // whether imported or not, to deal with C's loose rules around enums.
+  // This covers both frozen and non-frozen @objc enums.
+  if (isObjC())
+    return false;
+
+  // Otherwise, the only non-exhaustive cases are those that don't have a fixed
+  // layout.
+  assert(isFormallyExhaustive(M) == !isResilient(M,ResilienceExpansion::Maximal)
+         && "ignoring the effects of @inlinable, @testable, and @objc, "
+            "these should match up");
+  return !isResilient(M, expansion);
+}
+
 ProtocolDecl::ProtocolDecl(DeclContext *DC, SourceLoc ProtocolLoc,
                            SourceLoc NameLoc, Identifier Name,
                            MutableArrayRef<TypeLoc> Inherited,

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -421,7 +421,8 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
       os << ", " << customName
          << ", \"" << ED->getName() << "\"";
     }
-    os << ", " << (ED->isExhaustive(/*useDC*/nullptr) ? "closed" : "open")
+    os << ", "
+       << (ED->isFormallyExhaustive(/*useDC*/nullptr) ? "closed" : "open")
        << ") {\n";
 
     for (auto Elt : ED->getAllElements()) {

lib/SIL/SILInstructions.cpp

@@ -1425,10 +1425,11 @@ namespace {
     EnumDecl *decl = enumType.getEnumOrBoundGenericEnum();
     assert(decl && "switch_enum operand is not an enum");
 
-    // FIXME: Get expansion from SILFunction
-    if (decl->isResilient(inst->getModule().getSwiftModule(),
-                          ResilienceExpansion::Maximal))
+    const SILFunction *F = inst->getFunction();
+    if (!decl->isEffectivelyExhaustive(F->getModule().getSwiftModule(),
+                                       F->getResilienceExpansion())) {
       return nullptr;
+    }
 
     llvm::SmallPtrSet<EnumElementDecl *, 4> unswitchedElts;
     for (auto elt : decl->getAllElements())

lib/SIL/SILVerifier.cpp

@@ -3699,8 +3699,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
     // Find the set of enum elements for the type so we can verify
     // exhaustiveness.
-    // FIXME: We also need to consider if the enum is resilient, in which case
-    // we're never guaranteed to be exhaustive.
     llvm::DenseSet<EnumElementDecl*> unswitchedElts;
     eDecl->getAllElements(unswitchedElts);
 
@@ -3723,7 +3721,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     }
 
     // If the select is non-exhaustive, we require a default.
-    require(unswitchedElts.empty() || I->hasDefault(),
+    bool isExhaustive =
+        eDecl->isEffectivelyExhaustive(F.getModule().getSwiftModule(),
+                                       F.getResilienceExpansion());
+    require((isExhaustive && unswitchedElts.empty()) || I->hasDefault(),
             "nonexhaustive select_enum must have a default destination");
     if (I->hasDefault()) {
       requireSameType(I->getDefaultResult()->getType(),
@@ -3891,7 +3892,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     }
 
     // If the switch is non-exhaustive, we require a default.
-    require(unswitchedElts.empty() || SOI->hasDefault(),
+    bool isExhaustive =
+        uDecl->isEffectivelyExhaustive(F.getModule().getSwiftModule(),
+                                       F.getResilienceExpansion());
+    require((isExhaustive && unswitchedElts.empty()) || SOI->hasDefault(),
             "nonexhaustive switch_enum must have a default destination");
     if (SOI->hasDefault()) {
       // When SIL ownership is enabled, we require all default branches to take

lib/SILGen/SILGenDecl.cpp

@@ -752,6 +752,52 @@ class EnumElementPatternInitialization : public RefutablePatternInitialization {
 };
 } // end anonymous namespace
 
+/// If \p elt belongs to an enum that has exactly two cases and that can be
+/// exhaustively switched, return the other case. Otherwise, return nullptr.
+static EnumElementDecl *getOppositeBinaryDecl(const SILGenFunction &SGF,
+                                              const EnumElementDecl *elt) {
+  const EnumDecl *enumDecl = elt->getParentEnum();
+  if (!enumDecl->isEffectivelyExhaustive(SGF.SGM.SwiftModule,
+                                         SGF.F.getResilienceExpansion())) {
+    return nullptr;
+  }
+
+  EnumDecl::ElementRange range = enumDecl->getAllElements();
+  auto iter = range.begin();
+  if (iter == range.end())
+    return nullptr;
+  bool seenDecl = false;
+  EnumElementDecl *result = nullptr;
+  if (*iter == elt) {
+    seenDecl = true;
+  } else {
+    result = *iter;
+  }
+
+  ++iter;
+  if (iter == range.end())
+    return nullptr;
+  if (seenDecl) {
+    assert(!result);
+    result = *iter;
+  } else {
+    if (elt != *iter)
+      return nullptr;
+    seenDecl = true;
+  }
+  ++iter;
+
+  // If we reach this point, we saw the decl we were looking for and one other
+  // case. If we have any additional cases, then we do not have a binary enum.
+  if (iter != range.end())
+    return nullptr;
+
+  // This is always true since we have already returned earlier nullptr if we
+  // did not see the decl at all.
+  assert(seenDecl);
+  return result;
+}
+
 void EnumElementPatternInitialization::emitEnumMatch(
     ManagedValue value, EnumElementDecl *eltDecl, Initialization *subInit,
     JumpDest failureDest, SILLocation loc, SILGenFunction &SGF) {
@@ -787,8 +833,7 @@ void EnumElementPatternInitialization::emitEnumMatch(
   // If we have a binary enum, do not emit a true default case. This ensures
   // that we do not emit a destroy_value on a .None.
   bool inferredBinaryEnum = false;
-  auto *enumDecl = value.getType().getEnumOrBoundGenericEnum();
-  if (auto *otherDecl = enumDecl->getOppositeBinaryDecl(eltDecl)) {
+  if (auto *otherDecl = getOppositeBinaryDecl(SGF, eltDecl)) {
     inferredBinaryEnum = true;
     switchBuilder.addCase(otherDecl, defaultBlock, nullptr, handler);
   } else {

lib/SILGen/SILGenPattern.cpp

@@ -1802,15 +1802,9 @@ CaseBlocks::CaseBlocks(
   // Check to see if the enum may have values beyond the cases we can see
   // at compile-time. This includes future cases (for resilient enums) and
   // random values crammed into C enums.
-  //
-  // Note: This relies on the fact that there are no "non-resilient" enums that
-  // are still non-exhaustive, except for @objc enums.
-  bool canAssumeExhaustive = !enumDecl->isObjC();
-  if (canAssumeExhaustive) {
-    canAssumeExhaustive =
-        !enumDecl->isResilient(SGF.SGM.SwiftModule,
-                               SGF.F.getResilienceExpansion());
-  }
+  bool canAssumeExhaustive =
+      enumDecl->isEffectivelyExhaustive(SGF.getModule().getSwiftModule(),
+                                        SGF.F.getResilienceExpansion());
   if (canAssumeExhaustive) {
     // Check that Sema didn't let any cases slip through. (This can happen
     // with @_downgrade_exhaustivity_check.)

lib/SILOptimizer/Transforms/SILCodeMotion.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "sil-codemotion"
 #include "swift/SILOptimizer/PassManager/Passes.h"
+#include "swift/AST/Module.h"
 #include "swift/Basic/BlotMapVector.h"
 #include "swift/SIL/SILBuilder.h"
 #include "swift/SIL/SILModule.h"
@@ -315,6 +316,13 @@ void BBEnumTagDataflowState::handlePredCondSelectEnum(CondBranchInst *CondBr) {
   // If the enum only has 2 values and its tag isn't the true branch, then we
   // know the true branch must be the other tag.
   if (EnumDecl *E = Operand->getType().getEnumOrBoundGenericEnum()) {
+    // We can't do this optimization on non-exhaustive enums.
+    const SILFunction *Fn = CondBr->getFunction();
+    bool IsExhaustive =
+        E->isEffectivelyExhaustive(Fn->getModule().getSwiftModule(),
+                                   Fn->getResilienceExpansion());
+    if (!IsExhaustive)
+      return;
     // Look for a single other element on this enum.
     EnumElementDecl *OtherElt = nullptr;
     for (EnumElementDecl *Elt : E->getAllElements()) {

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "sil-simplify-cfg"
+#include "swift/AST/Module.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SIL/InstructionUtils.h"
@@ -1574,7 +1575,13 @@ bool SimplifyCFG::simplifyCondBrBlock(CondBranchInst *BI) {
     auto Iter = AllElts.begin();
     EnumElementDecl *FirstElt = *Iter;
 
-    if (SEI->getNumCases() >= 1
+    // We can't do this optimization on non-exhaustive enums.
+    bool IsExhaustive =
+        E->isEffectivelyExhaustive(Fn.getModule().getSwiftModule(),
+                                   Fn.getResilienceExpansion());
+
+    if (IsExhaustive
+        && SEI->getNumCases() >= 1
         && SEI->getCase(0).first != FirstElt) {
       ++Iter;
 

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -1122,7 +1122,7 @@ namespace {
                                          constElemSpaces);
           });
 
-          if (!E->isExhaustive(DC)) {
+          if (!E->isFormallyExhaustive(DC)) {
             arr.push_back(Space::forUnknown(/*allowedButNotRequired*/false));
           } else if (!E->getAttrs().hasAttribute<FrozenAttr>()) {
             arr.push_back(Space::forUnknown(/*allowedButNotRequired*/true));