Skip to content

Commit e4810f5

Browse files
tshortlitshortli
committed
Sema: Fix unavailable enum element cases in derived hashable/equatable impls.
When deriving the `hash(into:)` and `==` witnesses for `Hashable`/`Equatable` enums, call `_diagnoseUnavailableCodeReached()` in the case bodies for unavailable enum elements. This is needed because the previously derived code would operate on the values associated with unavailable enum elements, which is illegal if the types of those associated values are also unavailable (these case bodies would have failed typechecking had they been hand-written). The new structure also more explicitly documents that reaching these cases is unexpected, since unavailable enum elements should not be instantiated at run time.
1 parent 5c25343 commit e4810f5

File tree

6 files changed

+326
-0
lines changed

6 files changed

+326
-0
lines changed

‎lib/Sema/DerivedConformanceEquatableHashable.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,13 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
176176
for (auto elt : enumDecl->getAllElements()) {
177177
++elementCount;
178178

179+
if (auto *unavailableElementCase =
180+
DerivedConformance::unavailableEnumElementCaseStmt(
181+
enumType, elt, eqDecl, /*subPatternCount=*/2)) {
182+
cases.push_back(unavailableElementCase);
183+
continue;
184+
}
185+
179186
// .<elt>(let l0, let l1, ...)
180187
SmallVector<VarDecl*, 3> lhsPayloadVars;
181188
auto lhsSubpattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'l', eqDecl,
@@ -692,6 +699,13 @@ deriveBodyHashable_enum_hasAssociatedValues_hashInto(
692699
// For each enum element, generate a case statement that binds the associated
693700
// values so that they can be fed to the hasher.
694701
for (auto elt : enumDecl->getAllElements()) {
702+
if (auto *unavailableElementCase =
703+
DerivedConformance::unavailableEnumElementCaseStmt(enumType, elt,
704+
hashIntoDecl)) {
705+
cases.push_back(unavailableElementCase);
706+
continue;
707+
}
708+
695709
// case .<elt>(let a0, let a1, ...):
696710
SmallVector<VarDecl*, 3> payloadVars;
697711
SmallVector<ASTNode, 3> statements;

‎lib/Sema/DerivedConformances.cpp‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,19 @@ DerivedConformance::createBuiltinCall(ASTContext &ctx,
493493
return call;
494494
}
495495

496+
CallExpr *DerivedConformance::createDiagnoseUnavailableCodeReachedCallExpr(
497+
ASTContext &ctx) {
498+
FuncDecl *diagnoseDecl = ctx.getDiagnoseUnavailableCodeReached();
499+
auto diagnoseDeclRefExpr =
500+
new (ctx) DeclRefExpr(diagnoseDecl, DeclNameLoc(), true);
501+
diagnoseDeclRefExpr->setType(diagnoseDecl->getInterfaceType());
502+
auto argList = ArgumentList::createImplicit(ctx, {});
503+
auto callExpr = CallExpr::createImplicit(ctx, diagnoseDeclRefExpr, argList);
504+
callExpr->setType(ctx.getNeverType());
505+
callExpr->setThrows(false);
506+
return callExpr;
507+
}
508+
496509
AccessorDecl *DerivedConformance::
497510
addGetterToReadOnlyDerivedProperty(VarDecl *property,
498511
Type propertyContextType) {
@@ -761,6 +774,13 @@ DeclRefExpr *DerivedConformance::convertEnumToIndex(SmallVectorImpl<ASTNode> &st
761774
unsigned index = 0;
762775
SmallVector<ASTNode, 4> cases;
763776
for (auto elt : enumDecl->getAllElements()) {
777+
if (auto *unavailableElementCase =
778+
DerivedConformance::unavailableEnumElementCaseStmt(enumType, elt,
779+
funcDecl)) {
780+
cases.push_back(unavailableElementCase);
781+
continue;
782+
}
783+
764784
// generate: case .<Case>:
765785
auto pat = new (C)
766786
EnumElementPattern(TypeExpr::createImplicit(enumType, C), SourceLoc(),
@@ -907,6 +927,52 @@ Pattern *DerivedConformance::enumElementPayloadSubpattern(
907927
return ParenPattern::createImplicit(C, letPattern);
908928
}
909929

930+
CaseStmt *DerivedConformance::unavailableEnumElementCaseStmt(
931+
Type enumType, EnumElementDecl *elt, DeclContext *parentDC,
932+
unsigned subPatternCount) {
933+
assert(subPatternCount > 0);
934+
935+
ASTContext &C = parentDC->getASTContext();
936+
auto availableAttr = elt->getAttrs().getUnavailable(C);
937+
if (!availableAttr)
938+
return nullptr;
939+
940+
if (!availableAttr->isUnconditionallyUnavailable())
941+
return nullptr;
942+
943+
auto createElementPattern = [&]() -> EnumElementPattern * {
944+
// .<elt>
945+
EnumElementPattern *eltPattern = new (C) EnumElementPattern(
946+
TypeExpr::createImplicit(enumType, C), SourceLoc(), DeclNameLoc(),
947+
DeclNameRef(elt->getBaseIdentifier()), elt, nullptr, /*DC*/ parentDC);
948+
eltPattern->setImplicit();
949+
return eltPattern;
950+
};
951+
952+
Pattern *labelItemPattern;
953+
if (subPatternCount > 1) {
954+
SmallVector<TuplePatternElt, 2> tuplePatternElts;
955+
for (unsigned i = 0; i < subPatternCount; i++) {
956+
tuplePatternElts.push_back(TuplePatternElt(createElementPattern()));
957+
}
958+
959+
// (.<elt>, ..., .<elt>)
960+
auto caseTuplePattern = TuplePattern::createImplicit(C, tuplePatternElts);
961+
caseTuplePattern->setImplicit();
962+
labelItemPattern = caseTuplePattern;
963+
} else {
964+
labelItemPattern = createElementPattern();
965+
}
966+
967+
auto labelItem = CaseLabelItem(labelItemPattern);
968+
auto *callExpr =
969+
DerivedConformance::createDiagnoseUnavailableCodeReachedCallExpr(C);
970+
auto body = BraceStmt::create(C, SourceLoc(), {callExpr}, SourceLoc());
971+
return CaseStmt::create(C, CaseParentKind::Switch, SourceLoc(), labelItem,
972+
SourceLoc(), SourceLoc(), body, {},
973+
/*implicit*/ true);
974+
}
975+
910976
/// Creates a named variable based on a prefix character and a numeric index.
911977
/// \p prefixChar The prefix character for the variable's name.
912978
/// \p index The numeric index to append to the variable's name.

‎lib/Sema/DerivedConformances.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class AssociatedTypeDecl;
2929
class ASTContext;
3030
struct ASTNode;
3131
class CallExpr;
32+
class CaseStmt;
3233
class Decl;
3334
class DeclContext;
3435
class DeclRefExpr;
@@ -389,6 +390,11 @@ class DerivedConformance {
389390
ArrayRef<ProtocolConformanceRef> conformances,
390391
ArrayRef<Expr *> args);
391392

393+
/// Build a call to the stdlib function that should be called when unavailable
394+
/// code is reached unexpectedly.
395+
static CallExpr *
396+
createDiagnoseUnavailableCodeReachedCallExpr(ASTContext &ctx);
397+
392398
/// Returns true if this derivation is trying to use a context that isn't
393399
/// appropriate for deriving.
394400
///
@@ -453,6 +459,16 @@ class DerivedConformance {
453459
EnumElementDecl *enumElementDecl, char varPrefix, DeclContext *varContext,
454460
SmallVectorImpl<VarDecl *> &boundVars, bool useLabels = false);
455461

462+
/// Creates a synthesized case statement that has the following structure:
463+
///
464+
/// case .<elt>, ..., .<elt>:
465+
/// _diagnoseUnavailableCodeReached()
466+
///
467+
/// The number of \c .<elt> matches is equal to \p subPatternCount.
468+
static CaseStmt *unavailableEnumElementCaseStmt(
469+
Type enumType, EnumElementDecl *enumElementDecl, DeclContext *parentDC,
470+
unsigned subPatternCount = 1);
471+
456472
static VarDecl *indexedVarDecl(char prefixChar, int index, Type type,
457473
DeclContext *varContext);
458474
};

‎test/Interpreter/enum_equatable_hashable_correctness.swift‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %target-run-simple-swift
2+
// RUN: %target-run-simple-swift(-Xfrontend -unavailable-decl-optimization=complete)
23
// REQUIRES: executable_test
34

45
import StdlibUnittest
@@ -17,6 +18,20 @@ enum Combo<T: Hashable, U: Hashable>: Hashable {
1718
case both(T, U)
1819
}
1920

21+
@available(*, unavailable)
22+
struct UnavailableStruct: Hashable {}
23+
24+
enum HasUnavailableCases: Hashable {
25+
case available
26+
case availablePayload(Int)
27+
28+
@available(*, unavailable)
29+
case unavailable
30+
31+
@available(*, unavailable)
32+
case unavailablePayload(UnavailableStruct)
33+
}
34+
2035
var EnumSynthesisTests = TestSuite("EnumSynthesis")
2136

2237
EnumSynthesisTests.test("BasicEquatability/Hashability") {
@@ -52,6 +67,13 @@ EnumSynthesisTests.test("CloseGenericValuesDoNotCollide") {
5267
expectNotEqual(Combo<String, Int>.both("foo", 3).hashValue, Combo<String, Int>.both("goo", 4).hashValue)
5368
}
5469

70+
EnumSynthesisTests.test("HasUnavailableCasesEquatability/Hashability") {
71+
checkHashable([
72+
HasUnavailableCases.available,
73+
HasUnavailableCases.availablePayload(2),
74+
], equalityOracle: { $0 == $1 })
75+
}
76+
5577
func hashEncode(_ body: (inout Hasher) -> ()) -> Int {
5678
var hasher = Hasher()
5779
body(&hasher)

‎test/decl/enum/derived_hashable_equatable.swift‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,133 @@ enum HasAssociatedValues: Hashable {
9090
// CHECK-NEXT: }
9191
// CHECK-NEXT: }
9292
}
93+
94+
// CHECK-LABEL: internal enum HasUnavailableElement : Hashable
95+
enum HasUnavailableElement: Hashable {
96+
// CHECK: case a
97+
case a
98+
// CHECK: @available(*, unavailable)
99+
// CHECK-NEXT: case b
100+
@available(*, unavailable)
101+
case b
102+
103+
// CHECK: @_implements(Equatable, ==(_:_:)) internal static func __derived_enum_equals(_ a: HasUnavailableElement, _ b: HasUnavailableElement) -> Bool {
104+
// CHECK-NEXT: private var index_a: Int
105+
// CHECK-NEXT: switch a {
106+
// CHECK-NEXT: case .a:
107+
// CHECK-NEXT: index_a = 0
108+
// CHECK-NEXT: case .b:
109+
// CHECK-NEXT: _diagnoseUnavailableCodeReached()
110+
// CHECK-NEXT: }
111+
// CHECK-NEXT: private var index_b: Int
112+
// CHECK-NEXT: switch b {
113+
// CHECK-NEXT: case .a:
114+
// CHECK-NEXT: index_b = 0
115+
// CHECK-NEXT: case .b:
116+
// CHECK-NEXT: _diagnoseUnavailableCodeReached()
117+
// CHECK-NEXT: }
118+
// CHECK-NEXT: return index_a == index_b
119+
// CHECK-NEXT: }
120+
121+
// CHECK: internal func hash(into hasher: inout Hasher) {
122+
// CHECK-NEXT: private var discriminator: Int
123+
// CHECK-NEXT: switch self {
124+
// CHECK-NEXT: case .a:
125+
// CHECK-NEXT: discriminator = 0
126+
// CHECK-NEXT: case .b:
127+
// CHECK-NEXT: _diagnoseUnavailableCodeReached()
128+
// CHECK-NEXT: }
129+
// CHECK-NEXT: hasher.combine(discriminator)
130+
// CHECK-NEXT: }
131+
132+
// CHECK: internal var hashValue: Int {
133+
// CHECK-NEXT: get {
134+
// CHECK-NEXT: return _hashValue(for: self)
135+
// CHECK-NEXT: }
136+
// CHECK-NEXT: }
137+
}
138+
139+
// CHECK-LABEL: internal enum HasAssociatedValuesAndUnavailableElement : Hashable
140+
enum HasAssociatedValuesAndUnavailableElement: Hashable {
141+
// CHECK: case a(Int)
142+
case a(Int)
143+
// CHECK: @available(*, unavailable)
144+
// CHECK-NEXT: case b(String)
145+
@available(*, unavailable)
146+
case b(String)
147+
148+
// CHECK: internal func hash(into hasher: inout Hasher) {
149+
// CHECK-NEXT: switch self {
150+
// CHECK-NEXT: case .a(let a0):
151+
// CHECK-NEXT: hasher.combine(0)
152+
// CHECK-NEXT: hasher.combine(a0)
153+
// CHECK-NEXT: case .b:
154+
// CHECK-NEXT: _diagnoseUnavailableCodeReached()
155+
// CHECK-NEXT: }
156+
// CHECK-NEXT: }
157+
158+
// CHECK: @_implements(Equatable, ==(_:_:)) internal static func __derived_enum_equals(_ a: HasAssociatedValuesAndUnavailableElement, _ b: HasAssociatedValuesAndUnavailableElement) -> Bool {
159+
// CHECK-NEXT: switch (a, b) {
160+
// CHECK-NEXT: case (.a(let l0), .a(let r0)):
161+
// CHECK-NEXT: guard l0 r0 else {
162+
// CHECK-NEXT: return false
163+
// CHECK-NEXT: }
164+
// CHECK-NEXT: return true
165+
// CHECK-NEXT: case (.b, .b):
166+
// CHECK-NEXT: _diagnoseUnavailableCodeReached()
167+
// CHECK-NEXT: default:
168+
// CHECK-NEXT: return false
169+
// CHECK-NEXT: }
170+
// CHECK-NEXT: }
171+
172+
// CHECK: internal var hashValue: Int {
173+
// CHECK-NEXT: get {
174+
// CHECK-NEXT: return _hashValue(for: self)
175+
// CHECK-NEXT: }
176+
// CHECK-NEXT: }
177+
}
178+
179+
// CHECK-LABEL: internal enum UnavailableEnum : Hashable
180+
@available(*, unavailable)
181+
enum UnavailableEnum: Hashable {
182+
// CHECK: case a
183+
case a
184+
// CHECK: case b
185+
case b
186+
187+
// CHECK: @_implements(Equatable, ==(_:_:)) internal static func __derived_enum_equals(_ a: UnavailableEnum, _ b: UnavailableEnum) -> Bool {
188+
// CHECK-NEXT: private var index_a: Int
189+
// CHECK-NEXT: switch a {
190+
// CHECK-NEXT: case .a:
191+
// CHECK-NEXT: index_a = 0
192+
// CHECK-NEXT: case .b:
193+
// CHECK-NEXT: index_a = 1
194+
// CHECK-NEXT: }
195+
// CHECK-NEXT: private var index_b: Int
196+
// CHECK-NEXT: switch b {
197+
// CHECK-NEXT: case .a:
198+
// CHECK-NEXT: index_b = 0
199+
// CHECK-NEXT: case .b:
200+
// CHECK-NEXT: index_b = 1
201+
// CHECK-NEXT: }
202+
// CHECK-NEXT: return index_a == index_b
203+
// CHECK-NEXT: }
204+
205+
// CHECK: internal func hash(into hasher: inout Hasher) {
206+
// CHECK-NEXT: private var discriminator: Int
207+
// CHECK-NEXT: switch self {
208+
// CHECK-NEXT: case .a:
209+
// CHECK-NEXT: discriminator = 0
210+
// CHECK-NEXT: case .b:
211+
// CHECK-NEXT: discriminator = 1
212+
// CHECK-NEXT: }
213+
// CHECK-NEXT: hasher.combine(discriminator)
214+
// CHECK-NEXT: }
215+
216+
// CHECK: internal var hashValue: Int {
217+
// CHECK-NEXT: get {
218+
// CHECK-NEXT: return _hashValue(for: self)
219+
// CHECK-NEXT: }
220+
// CHECK-NEXT: }
221+
222+
}

0 commit comments

Comments
 (0)