Merge pull request #79580 from xedin/extensible-enums

[Serialization/TypeChecker] Introduce `ExtensibleEnums` feature

Author: xedin

include/swift/AST/Decl.h

@@ -736,7 +736,7 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl>, public Swi
     HasAnyUnavailableDuringLoweringValues : 1
   );
 
-  SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+8,
+  SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+8,
     /// If the module is compiled as static library.
     StaticLibrary : 1,
 
@@ -805,7 +805,10 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl>, public Swi
     SerializePackageEnabled : 1,
 
     /// Whether this module has enabled strict memory safety checking.
-    StrictMemorySafety : 1
+    StrictMemorySafety : 1,
+
+    /// Whether this module has enabled `ExtensibleEnums` feature.
+    ExtensibleEnums : 1
   );
 
   SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,

include/swift/AST/Module.h

@@ -840,6 +840,14 @@ class ModuleDecl
     Bits.ModuleDecl.ObjCNameLookupCachePopulated = value;
   }
 
+  bool supportsExtensibleEnums() const {
+    return Bits.ModuleDecl.ExtensibleEnums;
+  }
+
+  void setSupportsExtensibleEnums(bool value = true) {
+    Bits.ModuleDecl.ExtensibleEnums = value;
+  }
+
   /// For the main module, retrieves the list of primary source files being
   /// compiled, that is, the files we're generating code for.
   ArrayRef<SourceFile *> getPrimarySourceFiles() const;

include/swift/Basic/Features.def

@@ -450,6 +450,11 @@ SUPPRESSIBLE_EXPERIMENTAL_FEATURE(CustomAvailability, true)
 /// Be strict about the Sendable conformance of metatypes.
 EXPERIMENTAL_FEATURE(StrictSendableMetatypes, true)
 
+/// Allow public enumerations to be extensible by default
+/// regardless of whether the module they are declared in
+/// is resilient or not.
+EXPERIMENTAL_FEATURE(ExtensibleEnums, true)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

include/swift/Serialization/Validation.h

@@ -150,7 +150,9 @@ class ExtendedValidationInfo {
     unsigned AllowNonResilientAccess: 1;
     unsigned SerializePackageEnabled: 1;
     unsigned StrictMemorySafety: 1;
+    unsigned SupportsExtensibleEnums : 1;
   } Bits;
+
 public:
   ExtendedValidationInfo() : Bits() {}
 
@@ -270,6 +272,11 @@ class ExtendedValidationInfo {
             version, SourceLoc(), /*Diags=*/nullptr))
       SwiftInterfaceCompilerVersion = genericVersion.value();
   }
+
+  bool supportsExtensibleEnums() const { return Bits.SupportsExtensibleEnums; }
+  void setSupportsExtensibleEnums(bool val) {
+    Bits.SupportsExtensibleEnums = val;
+  }
 };
 
 struct SearchPath {

lib/AST/Decl.cpp

@@ -6696,8 +6696,22 @@ bool EnumDecl::hasOnlyCasesWithoutAssociatedValues() const {
 }
 
 bool EnumDecl::treatAsExhaustiveForDiags(const DeclContext *useDC) const {
-  return isFormallyExhaustive(useDC) ||
-         (useDC && getModuleContext()->inSamePackage(useDC->getParentModule()));
+  if (useDC) {
+    auto *enumModule = getModuleContext();
+    if (enumModule->inSamePackage(useDC->getParentModule()))
+      return true;
+
+    // If the module where enum is declared supports extensible enumerations
+    // and this enum is not explicitly marked as "@frozen", cross-module
+    // access cannot be exhaustive and requires `@unknown default:`.
+    if (enumModule->supportsExtensibleEnums() &&
+        !getAttrs().hasAttribute<FrozenAttr>()) {
+      if (useDC != enumModule->getDeclContext())
+        return false;
+    }
+  }
+
+  return isFormallyExhaustive(useDC);
 }
 
 bool EnumDecl::isFormallyExhaustive(const DeclContext *useDC) const {

lib/AST/FeatureSet.cpp

@@ -122,6 +122,7 @@ UNINTERESTING_FEATURE(SuppressedAssociatedTypes)
 UNINTERESTING_FEATURE(StructLetDestructuring)
 UNINTERESTING_FEATURE(MacrosOnImports)
 UNINTERESTING_FEATURE(AsyncCallerExecution)
+UNINTERESTING_FEATURE(ExtensibleEnums)
 
 static bool usesFeatureNonescapableTypes(Decl *decl) {
   auto containsNonEscapable =

lib/Frontend/Frontend.cpp

@@ -1457,6 +1457,8 @@ ModuleDecl *CompilerInstance::getMainModule() const {
       MainModule->setSerializePackageEnabled();
     if (Invocation.getLangOptions().hasFeature(Feature::WarnUnsafe))
       MainModule->setStrictMemorySafety(true);
+    if (Invocation.getLangOptions().hasFeature(Feature::ExtensibleEnums))
+      MainModule->setSupportsExtensibleEnums(true);
 
     configureAvailabilityDomains(getASTContext(),
                                  Invocation.getFrontendOptions(), MainModule);

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -1154,13 +1154,19 @@ namespace {
         assert(defaultReason == RequiresDefault::No);
         Type subjectType = Switch->getSubjectExpr()->getType();
         bool shouldIncludeFutureVersionComment = false;
+        bool shouldDowngradeToWarning = true;
         if (auto *theEnum = subjectType->getEnumOrBoundGenericEnum()) {
+          auto *enumModule = theEnum->getParentModule();
           shouldIncludeFutureVersionComment =
-              theEnum->getParentModule()->isSystemModule();
+              enumModule->isSystemModule() ||
+              enumModule->supportsExtensibleEnums();
+          // Since the module enabled `ExtensibleEnums` feature they
+          // opted-in all of their clients into exhaustivity errors.
+          shouldDowngradeToWarning = !enumModule->supportsExtensibleEnums();
         }
         DE.diagnose(startLoc, diag::non_exhaustive_switch_unknown_only,
                     subjectType, shouldIncludeFutureVersionComment)
-          .warnUntilSwiftVersion(6);
+          .warnUntilSwiftVersionIf(shouldDowngradeToWarning, 6);
         mainDiagType = std::nullopt;
       }
         break;

lib/Serialization/ModuleFile.h

@@ -707,6 +707,11 @@ class ModuleFile
   /// \c true if this module was built with strict memory safety.
   bool strictMemorySafety() const { return Core->strictMemorySafety(); }
 
+  /// \c true if this module was built with `ExtensibleEnums` feature enabled.
+  bool supportsExtensibleEnums() const {
+    return Core->supportsExtensibleEnums();
+  }
+
   /// Associates this module file with the AST node representing it.
   ///
   /// Checks that the file is compatible with the AST module it's being loaded

lib/Serialization/ModuleFileSharedCore.cpp

@@ -224,6 +224,9 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor,
     case options_block::STRICT_MEMORY_SAFETY:
       extendedInfo.setStrictMemorySafety(true);
       break;
+    case options_block::EXTENSIBLE_ENUMS:
+      extendedInfo.setSupportsExtensibleEnums(true);
+      break;
     default:
       // Unknown options record, possibly for use by a future version of the
       // module format.
@@ -1501,6 +1504,7 @@ ModuleFileSharedCore::ModuleFileSharedCore(
       Bits.AllowNonResilientAccess = extInfo.allowNonResilientAccess();
       Bits.SerializePackageEnabled = extInfo.serializePackageEnabled();
       Bits.StrictMemorySafety = extInfo.strictMemorySafety();
+      Bits.SupportsExtensibleEnums = extInfo.supportsExtensibleEnums();
       MiscVersion = info.miscVersion;
       SDKVersion = info.sdkVersion;
       ModuleABIName = extInfo.getModuleABIName();

lib/Serialization/ModuleFileSharedCore.h

@@ -418,8 +418,11 @@ class ModuleFileSharedCore {
     /// Whether this module enabled strict memory safety.
     unsigned StrictMemorySafety : 1;
 
+    /// Whether this module enabled has `ExtensibleEnums` feature enabled.
+    unsigned SupportsExtensibleEnums : 1;
+
     // Explicitly pad out to the next word boundary.
-    unsigned : 2;
+    unsigned : 1;
   } Bits = {};
   static_assert(sizeof(ModuleBits) <= 8, "The bit set should be small");
 
@@ -678,6 +681,8 @@ class ModuleFileSharedCore {
 
   bool strictMemorySafety() const { return Bits.StrictMemorySafety; }
 
+  bool supportsExtensibleEnums() const { return Bits.SupportsExtensibleEnums; }
+
   /// How should \p dependency be loaded for a transitive import via \c this?
   ///
   /// If \p importNonPublicDependencies, more transitive dependencies

lib/Serialization/ModuleFormat.h

@@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 923; // debug values
+const uint16_t SWIFTMODULE_VERSION_MINOR = 924; // ExtensibleEnums feature
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -974,6 +974,7 @@ namespace options_block {
     PUBLIC_MODULE_NAME,
     SWIFT_INTERFACE_COMPILER_VERSION,
     STRICT_MEMORY_SAFETY,
+    EXTENSIBLE_ENUMS,
   };
 
   using SDKPathLayout = BCRecordLayout<
@@ -1083,6 +1084,10 @@ namespace options_block {
     SWIFT_INTERFACE_COMPILER_VERSION,
     BCBlob // version tuple
   >;
+
+  using ExtensibleEnumsLayout = BCRecordLayout<
+    EXTENSIBLE_ENUMS
+  >;
 }
 
 /// The record types within the input block.

lib/Serialization/Serialization.cpp

@@ -1181,6 +1181,11 @@ void Serializer::writeHeader() {
                            static_cast<uint8_t>(M->getCXXStdlibKind()));
       }
 
+      if (M->supportsExtensibleEnums()) {
+        options_block::ExtensibleEnumsLayout ExtensibleEnums(Out);
+        ExtensibleEnums.emit(ScratchRecord);
+      }
+
       if (Options.SerializeOptionsForDebugging) {
         options_block::SDKPathLayout SDKPath(Out);
         options_block::XCCLayout XCC(Out);

lib/Serialization/SerializedModuleLoader.cpp

@@ -1090,6 +1090,8 @@ LoadedFile *SerializedModuleLoaderBase::loadAST(
     if (!loadedModuleFile->getModulePackageName().empty()) {
       M.setPackageName(Ctx.getIdentifier(loadedModuleFile->getModulePackageName()));
     }
+    if (loadedModuleFile->supportsExtensibleEnums())
+      M.setSupportsExtensibleEnums();
     M.setUserModuleVersion(loadedModuleFile->getUserModuleVersion());
     M.setSwiftInterfaceCompilerVersion(
         loadedModuleFile->getSwiftInterfaceCompilerVersion());

test/ModuleInterface/extensible_enums.swift

@@ -0,0 +1,110 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/src)
+// RUN: split-file %s %t/src
+
+/// Build the library
+// RUN: %target-swift-frontend -emit-module %t/src/Lib.swift \
+// RUN:   -module-name Lib \
+// RUN:   -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -enable-experimental-feature ExtensibleEnums
+
+// Check that the errors are produced when using enums from module with `ExtensibleEnums` feature enabled.
+// RUN: %target-swift-frontend -typecheck %t/src/TestChecking.swift \
+// RUN:   -swift-version 5 -module-name Client -I %t \
+// RUN:   -verify
+
+// Test to make sure that if the library and client are in the same package enums are checked exhaustively
+
+/// Build the library
+// RUN: %target-swift-frontend -emit-module %t/src/Lib.swift \
+// RUN:   -module-name Lib \
+// RUN:   -package-name Test \
+// RUN:   -emit-module-path %t/Lib.swiftmodule \
+// RUN:   -enable-experimental-feature ExtensibleEnums
+
+// Different module but the same package
+// RUN: %target-swift-frontend -typecheck %t/src/TestSamePackage.swift \
+// RUN:   -swift-version 5 -module-name Client -I %t \
+// RUN:   -package-name Test \
+// RUN:   -verify
+
+// REQUIRES: swift_feature_ExtensibleEnums
+
+//--- Lib.swift
+
+public enum E {
+  case a
+}
+
+@frozen
+public enum F {
+  case a
+  case b
+}
+
+func test_same_module(e: E, f: F) {
+  switch e { // Ok
+  case .a: break 
+  }
+
+  switch f { // Ok
+  case .a: break
+  case .b: break
+  }
+}
+
+//--- TestChecking.swift
+import Lib
+
+func test(e: E, f: F) {
+  // `E` is not marked as `@frozen` which means it gets new semantics
+
+  switch e {
+  // expected-error@-1 {{switch covers known cases, but 'E' may have additional unknown values, possibly added in future versions}}
+  // expected-note@-2 {{handle unknown values using "@unknown default"}}
+  case .a: break
+  }
+
+  switch e { // Ok (no warnings)
+  case .a: break
+  @unknown default: break
+  }
+
+  // `F` is marked as `@frozen` which means regular rules apply even with `ExtensibleEnums` feature enabled.  
+
+  switch f { // Ok (no errors because `F` is `@frozen`)
+  case .a: break
+  case .b: break
+  }
+
+  switch f { // expected-error {{switch must be exhaustive}} expected-note {{dd missing case: '.b'}}
+  case .a: break
+  }
+  
+  switch f { // expected-warning {{switch must be exhaustive}} expected-note {{dd missing case: '.b'}}
+  case .a: break
+  @unknown default: break
+  }
+}
+
+//--- TestSamePackage.swift
+import Lib
+
+func test_no_default(e: E, f: F) {
+  switch e { // Ok
+  case .a: break
+  }
+
+  switch e { // expected-warning {{switch must be exhaustive}} expected-note {{dd missing case: '.a'}}
+  @unknown default: break
+  }
+
+  switch f { // expected-error {{switch must be exhaustive}} expected-note {{dd missing case: '.b'}}
+  case .a: break
+  }
+
+  switch f { // expected-warning {{switch must be exhaustive}} expected-note {{dd missing case: '.b'}}
+  case .a: break
+  @unknown default: break
+  }
+}