Skip to content

[Serialization/TypeChecker] Introduce ExtensibleEnums feature #79580

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.

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 3 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions include/swift/Serialization/Validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ class ExtendedValidationInfo {
unsigned AllowNonResilientAccess: 1;
unsigned SerializePackageEnabled: 1;
unsigned StrictMemorySafety: 1;
unsigned SupportsExtensibleEnums : 1;
} Bits;

public:
ExtendedValidationInfo() : Bits() {}

Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 16 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions lib/Sema/TypeCheckSwitchStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions lib/Serialization/ModuleFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/Serialization/ModuleFileSharedCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 6 additions & 1 deletion lib/Serialization/ModuleFileSharedCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/Serialization/ModuleFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -974,6 +974,7 @@ namespace options_block {
PUBLIC_MODULE_NAME,
SWIFT_INTERFACE_COMPILER_VERSION,
STRICT_MEMORY_SAFETY,
EXTENSIBLE_ENUMS,
};

using SDKPathLayout = BCRecordLayout<
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions lib/Serialization/SerializedModuleLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
110 changes: 110 additions & 0 deletions test/ModuleInterface/extensible_enums.swift
Original file line number Diff line number Diff line change
@@ -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'}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test that exhaustively switches over f but also has an @unknown default here. That should generate a warning as well since the default isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into that in a separate PR. Regular exhaustivity rules apply here, so what ever that implementation does is going to be reflected.

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
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for this

  switch e { // expected-warning
  case .a: break
  @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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here can we add a test for this

  switch f { // expected-warning
  case .a: break
  case .b: break
  @unknown default: break

}