Skip to content

Commit 7c0a9cc

Browse files
authored
Merge pull request #67636 from tshortli/enum-unavailable-element-derived-equatable-hashable
Sema: Fix unavailable enum element cases in derived hashable/equatable impls
2 parents bb13518 + e4810f5 commit 7c0a9cc

15 files changed

+453
-18
lines changed

lib/AST/ASTPrinter.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5659,9 +5659,10 @@ void PrintAST::visitCaseStmt(CaseStmt *CS) {
56595659
[&] { Printer << ", "; });
56605660
}
56615661
Printer << ":";
5662-
Printer.printNewline();
56635662

5664-
printASTNodes((cast<BraceStmt>(CS->getBody())->getElements()));
5663+
if (!printASTNodes((cast<BraceStmt>(CS->getBody())->getElements())))
5664+
Printer.printNewline();
5665+
indent();
56655666
}
56665667

56675668
void PrintAST::visitFailStmt(FailStmt *stmt) {

lib/Sema/DerivedConformanceComparable.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,5 +342,7 @@ void DerivedConformance::tryDiagnoseFailedComparableDerivation(
342342
rawType, nominal->getDeclaredInterfaceType(),
343343
comparableProto->getDeclaredInterfaceType());
344344
}
345+
// FIXME: Diagnose potentially unavailable enum elements that are preventing
346+
// Comparable synthesis.
345347
}
346348
}

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
};

lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -828,8 +828,8 @@ namespace {
828828
auto children = E->getAllElements();
829829
llvm::transform(
830830
children, std::back_inserter(arr), [&](EnumElementDecl *eed) {
831-
// Don't force people to match unavailable cases; they can't
832-
// even write them.
831+
// Don't force people to match unavailable cases since they
832+
// should not be instantiated at run time.
833833
if (AvailableAttr::isUnavailable(eed)) {
834834
return Space();
835835
}

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)

0 commit comments

Comments
 (0)