Skip to content

Implementation for /most/ of SE-0192 (frozen and non-frozen enums) #14945

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 12 commits into from
Mar 21, 2018
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
4 changes: 3 additions & 1 deletion include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ SIMPLE_DECL_ATTR(nonobjc, NonObjC,
OnExtension | OnFunc | OnVar | OnSubscript | OnConstructor, 30)

SIMPLE_DECL_ATTR(_fixed_layout, FixedLayout,
OnVar | OnClass | OnStruct | OnEnum | UserInaccessible, 31)
OnVar | OnClass | OnStruct | UserInaccessible, 31)

SIMPLE_DECL_ATTR(_inlineable, Inlineable,
OnVar | OnSubscript | OnFunc | OnConstructor | OnDestructor |
Expand Down Expand Up @@ -314,6 +314,8 @@ SIMPLE_DECL_ATTR(_weakLinked, WeakLinked,
UserInaccessible,
75)

SIMPLE_DECL_ATTR(_frozen, Frozen, OnEnum | UserInaccessible, 76)

#undef TYPE_ATTR
#undef DECL_ATTR_ALIAS
#undef SIMPLE_DECL_ATTR
Expand Down
7 changes: 7 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3238,6 +3238,13 @@ class EnumDecl final : public NominalTypeDecl {
bool isIndirect() const {
return getAttrs().hasAttribute<IndirectAttr>();
}

/// True if the enum can be exhaustively switched within \p useDC.
///
/// 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;
};

/// StructDecl - This is the declaration of a struct, for example:
Expand Down
6 changes: 5 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1204,11 +1204,15 @@ WARNING(expr_dynamic_lookup_swift3_objc_inference,none,
ERROR(alignment_not_power_of_two,none,
"alignment value must be a power of two", ())

// Indirect enums
// Enum annotations
ERROR(indirect_case_without_payload,none,
"enum case %0 without associated value cannot be 'indirect'", (Identifier))
ERROR(indirect_case_in_indirect_enum,none,
"enum case in 'indirect' enum cannot also be 'indirect'", ())
WARNING(enum_frozen_nonresilient,none,
"%0 has no effect without -enable-resilience", (DeclAttribute))
WARNING(enum_frozen_nonpublic,none,
"%0 has no effect on non-public enums", (DeclAttribute))

// Variables (var and let).
ERROR(unimplemented_static_var,none,
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ namespace swift {
/// Diagnose uses of NSCoding with classes that have unstable mangled names.
bool EnableNSKeyedArchiverDiagnostics = true;

/// Diagnose switches over non-frozen enums that do not have catch-all
/// cases.
bool EnableNonFrozenEnumExhaustivityDiagnostics = false;
Copy link
Member

Choose a reason for hiding this comment

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

This is for staging while we settle on the "unknown case" spelling, right? It'll go away then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's mainly so I can land this part before SE-0192's review period ends, but yes.


/// Regex for the passes that should report passed and missed optimizations.
///
/// These are shared_ptrs so that this class remains copyable.
Expand Down
7 changes: 7 additions & 0 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,13 @@ def disable_nskeyedarchiver_diagnostics :
Flag<["-"], "disable-nskeyedarchiver-diagnostics">,
HelpText<"Allow classes with unstable mangled names to adopt NSCoding">;

def enable_nonfrozen_enum_exhaustivity_diagnostics :
Flag<["-"], "enable-nonfrozen-enum-exhaustivity-diagnostics">,
HelpText<"Diagnose switches over non-frozen enums without catch-all cases">;
def disable_nonfrozen_enum_exhaustivity_diagnostics :
Flag<["-"], "disable-nonfrozen-enum-exhaustivity-diagnostics">,
HelpText<"Allow switches over non-frozen enums without catch-all cases">;

def warn_long_function_bodies : Separate<["-"], "warn-long-function-bodies">,
MetaVarName<"<n>">,
HelpText<"Warns when type-checking a function takes longer than <n> ms">;
Expand Down
4 changes: 2 additions & 2 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,9 +757,9 @@ namespace {

if (NTD->hasInterfaceType()) {
if (NTD->isResilient())
OS << " @_resilient_layout";
OS << " resilient";
else
OS << " @_fixed_layout";
OS << " non-resilient";
}
}

Expand Down
53 changes: 51 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2286,9 +2286,11 @@ bool NominalTypeDecl::isFormallyResilient() const {
/*respectVersionedAttr=*/true).isPublic())
return false;

// Check for an explicit @_fixed_layout attribute.
if (getAttrs().hasAttribute<FixedLayoutAttr>())
// Check for an explicit @_fixed_layout or @_frozen attribute.
if (getAttrs().hasAttribute<FixedLayoutAttr>() ||
getAttrs().hasAttribute<FrozenAttr>()) {
return false;
}

// Structs and enums imported from C *always* have a fixed layout.
// We know their size, and pass them as values in SIL and IRGen.
Expand Down Expand Up @@ -3088,6 +3090,53 @@ bool EnumDecl::hasOnlyCasesWithoutAssociatedValues() const {
return true;
}

bool EnumDecl::isExhaustive(const DeclContext *useDC) const {
// Enums explicitly marked frozen are exhaustive.
if (getAttrs().hasAttribute<FrozenAttr>())
return true;

// Objective-C enums /not/ marked frozen are /not/ exhaustive.
// Note: This implicitly holds @objc enums defined in Swift to a higher
// standard!
if (hasClangNode())
return false;

// Non-imported enums in non-resilient modules are exhaustive.
const ModuleDecl *containingModule = getModuleContext();
switch (containingModule->getResilienceStrategy()) {
case ResilienceStrategy::Default:
return true;
case ResilienceStrategy::Resilient:
break;
}

// Non-public, non-versioned enums are always exhaustive.
AccessScope accessScope = getFormalAccessScope(/*useDC*/nullptr,
/*respectVersioned*/true);
if (!accessScope.isPublic())
return true;

// All other checks are use-site specific; with no further information, the
// enum must be treated non-exhaustively.
if (!useDC)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we ever need to check exhaustiveness in SILGen or anything else that runs after Sema? Because we won't have a useDC anymore

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 think we'll have a DC up through SILGen, but you do have me worried about optimizations. Unfortunately we don't actually the right API for that ("AvailabilityContext"); fortunately, it'll err on the side of "non-exhaustive". (That is, passing nullptr or the containing module or the containing source file might answer "non-exhaustive" when it could be "exhaustive", but never the other way around.)

return false;

// Enums in the same module as the use site are exhaustive /unless/ the use
// site is inlinable.
if (useDC->getParentModule() == containingModule)
if (useDC->getResilienceExpansion() == ResilienceExpansion::Maximal)
return true;

// Testably imported enums are exhaustive, on the grounds that only the author
// of the original library can import it testably.
if (auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext()))
if (useSF->hasTestableImport(containingModule))
return true;

// Otherwise, the enum is non-exhaustive.
return false;
}

ProtocolDecl::ProtocolDecl(DeclContext *DC, SourceLoc ProtocolLoc,
SourceLoc NameLoc, Identifier Name,
MutableArrayRef<TypeLoc> Inherited,
Expand Down
23 changes: 18 additions & 5 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2653,7 +2653,8 @@ namespace {
break;
}

case EnumKind::Enum: {
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum: {
auto &C = Impl.SwiftContext;
EnumDecl *nativeDecl;
bool declaredNative = hasNativeSwiftDecl(decl, name, dc, nativeDecl);
Expand Down Expand Up @@ -2757,6 +2758,14 @@ namespace {
Impl.importSourceLoc(decl->getLocation()), None, nullptr, enumDC);
enumDecl->computeType();

// Annotate as 'frozen' if appropriate.
assert((DeclAttribute::getOptions(DAK_Frozen) &
DeclAttribute::UserInaccessible) &&
"Once 'frozen' is supported, the attribute should not be "
"implicit (below)");
if (enumKind == EnumKind::FrozenEnum)
enumDecl->getAttrs().add(new (C) FrozenAttr(/*implicit*/true));

// Set up the C underlying type as its Swift raw type.
enumDecl->setRawType(underlyingType);

Expand Down Expand Up @@ -2860,7 +2869,8 @@ namespace {
addEnumeratorsAsMembers = false;
break;
case EnumKind::Options:
case EnumKind::Enum:
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum:
addEnumeratorsAsMembers = true;
break;
}
Expand All @@ -2870,7 +2880,8 @@ namespace {
EnumElementDecl *>, 8,
APSIntRefDenseMapInfo> canonicalEnumConstants;

if (enumKind == EnumKind::Enum) {
if (enumKind == EnumKind::NonFrozenEnum ||
enumKind == EnumKind::FrozenEnum) {
for (auto constant : decl->enumerators()) {
if (Impl.isUnavailableInSwift(constant))
continue;
Expand Down Expand Up @@ -2931,7 +2942,8 @@ namespace {
return true;
});
break;
case EnumKind::Enum: {
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum: {
auto canonicalCaseIter =
canonicalEnumConstants.find(&constant->getInitVal());

Expand Down Expand Up @@ -3379,7 +3391,8 @@ namespace {
return result;
}

case EnumKind::Enum:
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum:
case EnumKind::Options: {
// The enumeration was mapped to a high-level Swift type, and its
// elements were created as children of that enum. They aren't available
Expand Down
73 changes: 63 additions & 10 deletions lib/ClangImporter/ImportEnumInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,55 @@ STATISTIC(EnumInfoNumCacheMisses, "# of times the enum info cache was missed");
using namespace swift;
using namespace importer;

static void rememberToChangeThisBehaviorInSwift5() {
// Note: Once the compiler starts advertising itself as Swift 5, even
// Swift 4 mode is supposed to treat C enums as non-exhaustive. Because
// it's Swift 4 mode, failing to switch over the whole enum will only
// produce a warning, not an error.
//
// This is an assertion rather than a condition because we /want/ to be
// reminded to take it out when we're ready for the Swift 5 release.
assert(version::getSwiftNumericVersion().first < 5 &&
"When the compiler starts advertising itself as Swift 5, even "
"Swift 4 mode is supposed to treat C enums as non-exhaustive.");
}

/// Find the last extensibility attribute on \p decl as arranged by source
/// location...unless there's an API note, in which case that one wins.
///
/// This is not what Clang will do, but it's more useful for us since CF_ENUM
/// already has enum_extensibility(open) in it.
static clang::EnumExtensibilityAttr *
getBestExtensibilityAttr(clang::Preprocessor &pp, const clang::EnumDecl *decl) {
clang::EnumExtensibilityAttr *bestSoFar = nullptr;
const clang::SourceManager &sourceMgr = pp.getSourceManager();
for (auto *next : decl->specific_attrs<clang::EnumExtensibilityAttr>()) {
if (next->getLocation().isInvalid()) {
// This is from API notes -- use it!
return next;
}

// Temporarily ignore enum_extensibility attributes inside CF_ENUM and
// similar. In the Swift 5 release we can start respecting this annotation,
// meaning this entire block can be dropped.
{
rememberToChangeThisBehaviorInSwift5();
auto loc = next->getLocation();
if (loc.isMacroID() &&
pp.getImmediateMacroName(loc) == "__CF_ENUM_ATTRIBUTES") {
continue;
}
}

if (!bestSoFar ||
sourceMgr.isBeforeInTranslationUnit(bestSoFar->getLocation(),
next->getLocation())) {
bestSoFar = next;
}
}
return bestSoFar;
}

/// Classify the given Clang enumeration to describe how to import it.
void EnumInfo::classifyEnum(const clang::EnumDecl *decl,
clang::Preprocessor &pp) {
Expand All @@ -49,21 +98,24 @@ void EnumInfo::classifyEnum(const clang::EnumDecl *decl,
return;
}

// First, check for attributes that denote the classification
// First, check for attributes that denote the classification.
if (auto domainAttr = decl->getAttr<clang::NSErrorDomainAttr>()) {
kind = EnumKind::Enum;
kind = EnumKind::NonFrozenEnum;
nsErrorDomain = domainAttr->getErrorDomain()->getName();
return;
}
if (decl->hasAttr<clang::FlagEnumAttr>()) {
kind = EnumKind::Options;
return;
}
if (decl->hasAttr<clang::EnumExtensibilityAttr>()) {
// FIXME: Distinguish between open and closed enums.
kind = EnumKind::Enum;
if (auto *attr = getBestExtensibilityAttr(pp, decl)) {
if (attr->getExtensibility() == clang::EnumExtensibilityAttr::Closed)
kind = EnumKind::FrozenEnum;
else
kind = EnumKind::NonFrozenEnum;
return;
}
if (!nsErrorDomain.empty())
return;

// If API notes have /removed/ a FlagEnum or EnumExtensibility attribute,
// then we don't need to check the macros.
Expand All @@ -79,15 +131,15 @@ void EnumInfo::classifyEnum(const clang::EnumDecl *decl,

// Was the enum declared using *_ENUM or *_OPTIONS?
// FIXME: Stop using these once flag_enum and enum_extensibility
// have been adopted everywhere, or at least relegate them to Swift 3 mode
// have been adopted everywhere, or at least relegate them to Swift 4 mode
// only.
auto loc = decl->getLocStart();
if (loc.isMacroID()) {
StringRef MacroName = pp.getImmediateMacroName(loc);
if (MacroName == "CF_ENUM" || MacroName == "__CF_NAMED_ENUM" ||
MacroName == "OBJC_ENUM" || MacroName == "SWIFT_ENUM" ||
MacroName == "SWIFT_ENUM_NAMED") {
kind = EnumKind::Enum;
kind = EnumKind::NonFrozenEnum;
return;
}
if (MacroName == "CF_OPTIONS" || MacroName == "OBJC_OPTIONS" ||
Expand All @@ -99,7 +151,7 @@ void EnumInfo::classifyEnum(const clang::EnumDecl *decl,

// Hardcode a particular annoying case in the OS X headers.
if (decl->getName() == "DYLD_BOOL") {
kind = EnumKind::Enum;
kind = EnumKind::FrozenEnum;
return;
}

Expand Down Expand Up @@ -205,7 +257,8 @@ StringRef importer::getCommonPluralPrefix(StringRef singular,
/// within the given enum.
void EnumInfo::determineConstantNamePrefix(const clang::EnumDecl *decl) {
switch (getKind()) {
case EnumKind::Enum:
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum:
case EnumKind::Options:
// Enums are mapped to Swift enums, Options to Swift option sets, both
// of which attempt prefix-stripping.
Expand Down
23 changes: 17 additions & 6 deletions lib/ClangImporter/ImportEnumInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ namespace importer {
/// into Swift. All of the possibilities have the same storage
/// representation, but can be used in different ways.
enum class EnumKind {
/// The enumeration type should map to an enum, which means that
/// all of the cases are independent.
Enum,
/// The enumeration type should map to an option set, which means
/// that
/// The enumeration type should map to a frozen enum, which means that
/// all of the cases are independent and there are no private cases.
FrozenEnum,
/// The enumeration type should map to a non-frozen enum, which means that
/// all of the cases are independent, but there may be values not represented
/// in the listed cases.
NonFrozenEnum,
/// The enumeration type should map to an option set, which means that
/// the constants represent combinations of independent flags.
Options,
/// The enumeration type should map to a distinct type, but we don't
Expand Down Expand Up @@ -76,7 +79,15 @@ class EnumInfo {

/// Whether this maps to an enum who also provides an error domain
bool isErrorEnum() const {
return getKind() == EnumKind::Enum && !nsErrorDomain.empty();
switch (getKind()) {
case EnumKind::FrozenEnum:
case EnumKind::NonFrozenEnum:
return !nsErrorDomain.empty();
case EnumKind::Options:
case EnumKind::Unknown:
case EnumKind::Constants:
return false;
}
}

/// For this error enum, extract the name of the error domain constant
Expand Down
6 changes: 4 additions & 2 deletions lib/ClangImporter/ImportName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,8 @@ NameImporter::determineEffectiveContext(const clang::NamedDecl *decl,
if (isa<clang::EnumConstantDecl>(decl)) {
auto enumDecl = cast<clang::EnumDecl>(dc);
switch (getEnumKind(enumDecl)) {
case EnumKind::Enum:
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum:
case EnumKind::Options:
// Enums are mapped to Swift enums, Options to Swift option sets.
if (version != ImportNameVersion::raw()) {
Expand Down Expand Up @@ -1005,7 +1006,8 @@ static bool shouldBeSwiftPrivate(NameImporter &nameImporter,
if (auto *ECD = dyn_cast<clang::EnumConstantDecl>(decl)) {
auto *ED = cast<clang::EnumDecl>(ECD->getDeclContext());
switch (nameImporter.getEnumKind(ED)) {
case EnumKind::Enum:
case EnumKind::NonFrozenEnum:
case EnumKind::FrozenEnum:
case EnumKind::Options:
if (version != ImportNameVersion::raw())
break;
Expand Down
Loading