Skip to content

Commit 6ea47bd

Browse files
committed
[ClangImporter] Prefer later enum_extensibility over earlier ones
Since NS_ENUM has an enum_extensibility(open) in it already, we want to allow people to undo that by sticking enum_extensibility(closed) on the end of their enum.
1 parent 52af3f3 commit 6ea47bd

File tree

6 files changed

+126
-3
lines changed

6 files changed

+126
-3
lines changed

lib/ClangImporter/ImportEnumInfo.cpp

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,55 @@ STATISTIC(EnumInfoNumCacheMisses, "# of times the enum info cache was missed");
3333
using namespace swift;
3434
using namespace importer;
3535

36+
static void rememberToChangeThisBehaviorInSwift5() {
37+
// Note: Once the compiler starts advertising itself as Swift 5, even
38+
// Swift 4 mode is supposed to treat C enums as non-exhaustive. Because
39+
// it's Swift 4 mode, failing to switch over the whole enum will only
40+
// produce a warning, not an error.
41+
//
42+
// This is an assertion rather than a condition because we /want/ to be
43+
// reminded to take it out when we're ready for the Swift 5 release.
44+
assert(version::getSwiftNumericVersion().first < 5 &&
45+
"When the compiler starts advertising itself as Swift 5, even "
46+
"Swift 4 mode is supposed to treat C enums as non-exhaustive.");
47+
}
48+
49+
/// Find the last extensibility attribute on \p decl as arranged by source
50+
/// location...unless there's an API note, in which case that one wins.
51+
///
52+
/// This is not what Clang will do, but it's more useful for us since CF_ENUM
53+
/// already has enum_extensibility(open) in it.
54+
static clang::EnumExtensibilityAttr *
55+
getBestExtensibilityAttr(clang::Preprocessor &pp, const clang::EnumDecl *decl) {
56+
clang::EnumExtensibilityAttr *bestSoFar = nullptr;
57+
const clang::SourceManager &sourceMgr = pp.getSourceManager();
58+
for (auto *next : decl->specific_attrs<clang::EnumExtensibilityAttr>()) {
59+
if (next->getLocation().isInvalid()) {
60+
// This is from API notes -- use it!
61+
return next;
62+
}
63+
64+
// Temporarily ignore enum_extensibility attributes inside CF_ENUM and
65+
// similar. In the Swift 5 release we can start respecting this annotation,
66+
// meaning this entire block can be dropped.
67+
{
68+
rememberToChangeThisBehaviorInSwift5();
69+
auto loc = next->getLocation();
70+
if (loc.isMacroID() &&
71+
pp.getImmediateMacroName(loc) == "__CF_ENUM_ATTRIBUTES") {
72+
continue;
73+
}
74+
}
75+
76+
if (!bestSoFar ||
77+
sourceMgr.isBeforeInTranslationUnit(bestSoFar->getLocation(),
78+
next->getLocation())) {
79+
bestSoFar = next;
80+
}
81+
}
82+
return bestSoFar;
83+
}
84+
3685
/// Classify the given Clang enumeration to describe how to import it.
3786
void EnumInfo::classifyEnum(const clang::EnumDecl *decl,
3887
clang::Preprocessor &pp) {
@@ -58,7 +107,7 @@ void EnumInfo::classifyEnum(const clang::EnumDecl *decl,
58107
kind = EnumKind::Options;
59108
return;
60109
}
61-
if (auto *attr = decl->getAttr<clang::EnumExtensibilityAttr>()) {
110+
if (auto *attr = getBestExtensibilityAttr(pp, decl)) {
62111
if (attr->getExtensibility() == clang::EnumExtensibilityAttr::Closed)
63112
kind = EnumKind::FrozenEnum;
64113
else
@@ -82,7 +131,7 @@ void EnumInfo::classifyEnum(const clang::EnumDecl *decl,
82131

83132
// Was the enum declared using *_ENUM or *_OPTIONS?
84133
// FIXME: Stop using these once flag_enum and enum_extensibility
85-
// have been adopted everywhere, or at least relegate them to Swift 3 mode
134+
// have been adopted everywhere, or at least relegate them to Swift 4 mode
86135
// only.
87136
auto loc = decl->getLocStart();
88137
if (loc.isMacroID()) {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Name: EnumExhaustivity
2+
Tags:
3+
- Name: RegularEnumTurnedExhaustiveThenBackViaAPINotes
4+
EnumExtensibility: open

test/ClangImporter/Inputs/custom-modules/EnumExhaustivity.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,31 @@ typedef MY_EXHAUSTIVE_ENUM(ExhaustiveEnum) {
1313
ExhaustiveEnumA,
1414
ExhaustiveEnumB
1515
};
16+
17+
typedef MY_ENUM(RegularEnumTurnedExhaustive) {
18+
RegularEnumTurnedExhaustiveA,
19+
RegularEnumTurnedExhaustiveB
20+
} __attribute__((enum_extensibility(closed)));
21+
22+
enum AnotherRegularEnumTurnedExhaustive {
23+
AnotherRegularEnumTurnedExhaustiveA,
24+
AnotherRegularEnumTurnedExhaustiveB
25+
} __attribute__((enum_extensibility(open))) __attribute__((enum_extensibility(closed)));
26+
27+
typedef MY_ENUM(RegularEnumTurnedExhaustiveThenBackViaAPINotes) {
28+
RegularEnumTurnedExhaustiveThenBackViaAPINotesA,
29+
RegularEnumTurnedExhaustiveThenBackViaAPINotesB
30+
} __attribute__((enum_extensibility(closed)));
31+
32+
typedef MY_ENUM(ForwardDeclaredTurnedExhaustive);
33+
enum ForwardDeclaredTurnedExhaustive {
34+
ForwardDeclaredTurnedExhaustiveA,
35+
ForwardDeclaredTurnedExhaustiveB
36+
} __attribute__((enum_extensibility(closed)));
37+
38+
enum __attribute__((enum_extensibility(open))) ForwardDeclaredOnly;
39+
enum __attribute__((enum_extensibility(closed))) ForwardDeclaredOnly;
40+
enum ForwardDeclaredOnly {
41+
ForwardDeclaredOnlyA,
42+
ForwardDeclaredOnlyB
43+
};

test/ClangImporter/Inputs/enum-inferred-exhaustivity.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,9 @@
88
typedef CF_ENUM(EnumWithDefaultExhaustivity) {
99
EnumWithDefaultExhaustivityLoneCase
1010
};
11+
12+
// This name is also specially recognized by Swift.
13+
#define __CF_ENUM_ATTRIBUTES __attribute__((enum_extensibility(open)))
14+
typedef CF_ENUM(EnumWithSpecialAttributes) {
15+
EnumWithSpecialAttributesLoneCase
16+
} __CF_ENUM_ATTRIBUTES;

test/ClangImporter/enum-exhaustivity.swift

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %target-swift-frontend -typecheck %s -I %S/Inputs/custom-modules -verify -warnings-as-errors -enable-nonfrozen-enum-exhaustivity-diagnostics
22

3-
// RUN: %target-swift-ide-test -source-filename %s -print-module -module-to-print EnumExhaustivity -I %S/Inputs/custom-modules | %FileCheck -check-prefix=CHECK %s
3+
// RUN: %target-swift-ide-test -source-filename %s -print-module -module-to-print EnumExhaustivity -I %S/Inputs/custom-modules | %FileCheck %s
44

55
// CHECK-LABEL: {{^}}enum RegularEnum : {{.+}} {
66
// CHECK: case A
@@ -13,6 +13,7 @@
1313
// CHECK-NEXT: case B
1414
// CHECK-NEXT: {{^}$}}
1515

16+
1617
import EnumExhaustivity
1718

1819
func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
@@ -26,3 +27,31 @@ func test(_ value: RegularEnum, _ exhaustiveValue: ExhaustiveEnum) {
2627
case .B: break
2728
}
2829
}
30+
31+
func testAttributes(
32+
_ rete: RegularEnumTurnedExhaustive,
33+
_ arete: AnotherRegularEnumTurnedExhaustive,
34+
_ retetb: RegularEnumTurnedExhaustiveThenBackViaAPINotes,
35+
_ fdte: ForwardDeclaredTurnedExhaustive,
36+
_ fdo: ForwardDeclaredOnly
37+
) {
38+
switch rete {
39+
case .A, .B: break
40+
}
41+
42+
switch arete {
43+
case .A, .B: break
44+
}
45+
46+
switch retetb { // expected-error {{switch must be exhaustive}} expected-note {{do you want to add a default clause?}}
47+
case .A, .B: break
48+
}
49+
50+
switch fdte {
51+
case .A, .B: break
52+
}
53+
54+
switch fdo {
55+
case .A, .B: break
56+
}
57+
}

test/ClangImporter/enum-inferred-exhaustivity.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,10 @@ func test(_ value: EnumWithDefaultExhaustivity) {
1010
case .loneCase: break
1111
}
1212
}
13+
14+
func test(_ value: EnumWithSpecialAttributes) {
15+
// Same, but with the attributes macro shipped in the Xcode 9 SDKs.
16+
switch value { // expected-error {{switch must be exhaustive}} expected-note {{do you want to add a default clause?}}
17+
case .loneCase: break
18+
}
19+
}

0 commit comments

Comments
 (0)