Skip to content

Commit feba547

Browse files
tshortlitshortli
committed
AST: Adopt Decl::isUnreachableAtRuntime() in Hashable/Equatable derivation.
When deriving `Hashable` and `Equatable` for enums, use `Decl::isUnreachableAtRuntime()` to determine whether or not to insert `_diagnoseUnavailableCodeReached()` traps for specific enum elements. This fixes a bug where inappropriate traps were inserted for enum elements that are unavailable for app extensions. It also fixes a bug where traps were inserted when building a zippered library for macOS and enum elements were unavailable on macOS but not for macCatalyst clients. Resolves rdar://125371621
1 parent cfb5114 commit feba547

File tree

5 files changed

+131
-11
lines changed

5 files changed

+131
-11
lines changed

‎include/swift/AST/Decl.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,11 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
13061306
std::optional<std::pair<const AvailableAttr *, const Decl *>>
13071307
getSemanticUnavailableAttr(bool ignoreAppExtensions = false) const;
13081308

1309+
/// Returns true if code associated with this declaration should be considerd
1310+
/// unreachable at runtime because the declaration is unavailable in all
1311+
/// execution contexts in which the code may run.
1312+
bool isUnreachableAtRuntime() const;
1313+
13091314
/// Returns true if this declaration should be considered available during
13101315
/// SIL/IR lowering. A declaration would not be available during lowering if,
13111316
/// for example, it is annotated as unavailable with `@available` and

‎lib/AST/Availability.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,13 @@ Decl::getSemanticUnavailableAttr(bool ignoreAppExtensions) const {
332332
std::nullopt);
333333
}
334334

335-
static bool shouldStubOrSkipUnavailableDecl(const Decl *D) {
335+
bool Decl::isUnreachableAtRuntime() const {
336336
// Don't trust unavailability on declarations from clang modules.
337-
if (isa<ClangModuleUnit>(D->getDeclContext()->getModuleScopeContext()))
337+
if (isa<ClangModuleUnit>(getDeclContext()->getModuleScopeContext()))
338338
return false;
339339

340340
auto unavailableAttrAndDecl =
341-
D->getSemanticUnavailableAttr(/*ignoreAppExtensions=*/true);
341+
getSemanticUnavailableAttr(/*ignoreAppExtensions=*/true);
342342
if (!unavailableAttrAndDecl)
343343
return false;
344344

@@ -358,7 +358,7 @@ static bool shouldStubOrSkipUnavailableDecl(const Decl *D) {
358358
// If we have a target variant (e.g. we're building a zippered macOS
359359
// framework) then the decl is only unreachable if it is unavailable for both
360360
// the primary target and the target variant.
361-
if (D->getASTContext().LangOpts.TargetVariant.has_value())
361+
if (getASTContext().LangOpts.TargetVariant.has_value())
362362
return false;
363363

364364
return true;
@@ -379,7 +379,7 @@ bool Decl::isAvailableDuringLowering() const {
379379
UnavailableDeclOptimization::Complete)
380380
return true;
381381

382-
return !shouldStubOrSkipUnavailableDecl(this);
382+
return !isUnreachableAtRuntime();
383383
}
384384

385385
bool Decl::requiresUnavailableDeclABICompatibilityStubs() const {
@@ -389,7 +389,7 @@ bool Decl::requiresUnavailableDeclABICompatibilityStubs() const {
389389
UnavailableDeclOptimization::Stub)
390390
return false;
391391

392-
return shouldStubOrSkipUnavailableDecl(this);
392+
return isUnreachableAtRuntime();
393393
}
394394

395395
bool UnavailabilityReason::requiresDeploymentTargetOrEarlier(

‎lib/Sema/DerivedConformances.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -932,11 +932,8 @@ CaseStmt *DerivedConformance::unavailableEnumElementCaseStmt(
932932
assert(subPatternCount > 0);
933933

934934
ASTContext &C = parentDC->getASTContext();
935-
auto availableAttr = elt->getAttrs().getUnavailable(C);
936-
if (!availableAttr)
937-
return nullptr;
938-
939-
if (!availableAttr->isUnconditionallyUnavailable())
935+
if (!elt->isUnreachableAtRuntime() ||
936+
elt->getParentEnum()->isUnreachableAtRuntime())
940937
return nullptr;
941938

942939
// If the stdlib isn't new enough to contain the helper function for

‎test/decl/enum/derived_hashable_equatable_macos.swift‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %target-swift-frontend -print-ast %s | %FileCheck %s --check-prefixes=CHECK,CHECK-PRE-SWIFT5_9
2+
// RUN: %target-swift-frontend -application-extension -print-ast %s | %FileCheck %s --check-prefixes=CHECK,CHECK-PRE-SWIFT5_9
23
// RUN: %target-swift-frontend -target %target-cpu-apple-macosx10.51 -print-ast %s | %FileCheck %s --check-prefixes=CHECK,CHECK-PRE-SWIFT5_9
34
// RUN: %target-swift-frontend -target %target-cpu-apple-macosx14 -print-ast %s | %FileCheck %s --check-prefixes=CHECK,CHECK-SWIFT5_9
45
// REQUIRES: OS=macosx
@@ -23,6 +24,10 @@ enum HasElementsWithAvailability: Hashable {
2324
// CHECK-NEXT: case introduced10_50
2425
@available(macOS, introduced: 10.50)
2526
case introduced10_50
27+
// CHECK: @available(macOSApplicationExtension, unavailable)
28+
// CHECK-NEXT: case unavailableMacOSAppExtension
29+
@available(macOSApplicationExtension, unavailable)
30+
case unavailableMacOSAppExtension
2631

2732
// CHECK: @_implements(Equatable, ==(_:_:)) internal static func __derived_enum_equals(_ a: HasElementsWithAvailability, _ b: HasElementsWithAvailability) -> Bool {
2833
// CHECK-NEXT: var index_a: Int
@@ -39,6 +44,8 @@ enum HasElementsWithAvailability: Hashable {
3944
// CHECK-NEXT: index_a = 1
4045
// CHECK-NEXT: case .introduced10_50:
4146
// CHECK-NEXT: index_a = 2
47+
// CHECK-NEXT: case .unavailableMacOSAppExtension:
48+
// CHECK-NEXT: index_a = 3
4249
// CHECK-NEXT: }
4350
// CHECK-NEXT: var index_b: Int
4451
// CHECK-NEXT: switch b {
@@ -54,6 +61,8 @@ enum HasElementsWithAvailability: Hashable {
5461
// CHECK-NEXT: index_b = 1
5562
// CHECK-NEXT: case .introduced10_50:
5663
// CHECK-NEXT: index_b = 2
64+
// CHECK-NEXT: case .unavailableMacOSAppExtension:
65+
// CHECK-NEXT: index_b = 3
5766
// CHECK-NEXT: }
5867
// CHECK-NEXT: return index_a == index_b
5968
// CHECK-NEXT: }
@@ -73,6 +82,8 @@ enum HasElementsWithAvailability: Hashable {
7382
// CHECK-NEXT: discriminator = 1
7483
// CHECK-NEXT: case .introduced10_50:
7584
// CHECK-NEXT: discriminator = 2
85+
// CHECK-NEXT: case .unavailableMacOSAppExtension:
86+
// CHECK-NEXT: discriminator = 3
7687
// CHECK-NEXT: }
7788
// CHECK-NEXT: hasher.combine(discriminator)
7889
// CHECK-NEXT: }

‎test/decl/enum/derived_hashable_equatable_macos_zippered.swift‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// RUN: %target-swift-frontend -target %target-cpu-apple-macosx13 -target-variant %target-cpu-apple-ios16-macabi -print-ast %s | %FileCheck %s
2+
// REQUIRES: OS=macosx
3+
4+
// CHECK-LABEL: internal enum HasElementsWithAvailability : Hashable
5+
enum HasElementsWithAvailability: Hashable {
6+
// CHECK: case alwaysAvailable
7+
case alwaysAvailable
8+
// CHECK: @available(*, unavailable)
9+
// CHECK-NEXT: case neverAvailable
10+
@available(*, unavailable)
11+
case neverAvailable
12+
// CHECK: @available(macOS, unavailable)
13+
// CHECK-NEXT: case unavailableMacOS
14+
@available(macOS, unavailable)
15+
case unavailableMacOS
16+
// CHECK: @available(iOS, unavailable)
17+
// CHECK-NEXT: case unavailableiOS
18+
@available(iOS, unavailable)
19+
case unavailableiOS
20+
// CHECK: @available(macCatalyst, unavailable)
21+
// CHECK-NEXT: case unavailableMacCatalyst
22+
@available(macCatalyst, unavailable)
23+
case unavailableMacCatalyst
24+
// CHECK: @available(macOS, unavailable)
25+
// CHECK-NEXT: @available(iOS, unavailable)
26+
// CHECK-NEXT: case unavailableMacOSAndiOS
27+
@available(macOS, unavailable)
28+
@available(iOS, unavailable)
29+
case unavailableMacOSAndiOS
30+
// CHECK: @available(macOS, unavailable)
31+
// CHECK-NEXT: @available(macCatalyst, unavailable)
32+
// CHECK-NEXT: case unavailableMacOSAndMacCatalyst
33+
@available(macOS, unavailable)
34+
@available(macCatalyst, unavailable)
35+
case unavailableMacOSAndMacCatalyst
36+
37+
// CHECK: @_implements(Equatable, ==(_:_:)) internal static func __derived_enum_equals(_ a: HasElementsWithAvailability, _ b: HasElementsWithAvailability) -> Bool {
38+
// CHECK-NEXT: var index_a: Int
39+
// CHECK-NEXT: switch a {
40+
// CHECK-NEXT: case .alwaysAvailable:
41+
// CHECK-NEXT: index_a = 0
42+
// CHECK-NEXT: case .neverAvailable:
43+
// CHECK-NEXT: _diagnoseUnavailableCodeReached{{.*}}
44+
// CHECK-NEXT: case .unavailableMacOS:
45+
// CHECK-NEXT: index_a = 1
46+
// CHECK-NEXT: case .unavailableiOS:
47+
// CHECK-NEXT: index_a = 2
48+
// CHECK-NEXT: case .unavailableMacCatalyst:
49+
// CHECK-NEXT: index_a = 3
50+
// FIXME: This case should diagnose (rdar://125930716)
51+
// CHECK-NEXT: case .unavailableMacOSAndiOS:
52+
// CHECK-NEXT: index_a = 4
53+
// FIXME: This case should diagnose (rdar://125930716)
54+
// CHECK-NEXT: case .unavailableMacOSAndMacCatalyst:
55+
// CHECK-NEXT: index_a = 5
56+
// CHECK-NEXT: }
57+
// CHECK-NEXT: var index_b: Int
58+
// CHECK-NEXT: switch b {
59+
// CHECK-NEXT: case .alwaysAvailable:
60+
// CHECK-NEXT: index_b = 0
61+
// CHECK-NEXT: case .neverAvailable:
62+
// CHECK-NEXT: _diagnoseUnavailableCodeReached{{.*}}
63+
// CHECK-NEXT: case .unavailableMacOS:
64+
// CHECK-NEXT: index_b = 1
65+
// CHECK-NEXT: case .unavailableiOS:
66+
// CHECK-NEXT: index_b = 2
67+
// CHECK-NEXT: case .unavailableMacCatalyst:
68+
// CHECK-NEXT: index_b = 3
69+
// FIXME: This case should diagnose (rdar://125930716)
70+
// CHECK-NEXT: case .unavailableMacOSAndiOS:
71+
// CHECK-NEXT: index_b = 4
72+
// FIXME: This case should diagnose (rdar://125930716)
73+
// CHECK-NEXT: case .unavailableMacOSAndMacCatalyst:
74+
// CHECK-NEXT: index_b = 5
75+
// CHECK-NEXT: }
76+
// CHECK-NEXT: return index_a == index_b
77+
// CHECK-NEXT: }
78+
79+
// CHECK: internal func hash(into hasher: inout Hasher) {
80+
// CHECK-NEXT: var discriminator: Int
81+
// CHECK-NEXT: switch self {
82+
// CHECK-NEXT: case .alwaysAvailable:
83+
// CHECK-NEXT: discriminator = 0
84+
// CHECK-NEXT: case .neverAvailable:
85+
// CHECK-NEXT: _diagnoseUnavailableCodeReached{{.*}}
86+
// CHECK-NEXT: case .unavailableMacOS:
87+
// CHECK-NEXT: discriminator = 1
88+
// CHECK-NEXT: case .unavailableiOS:
89+
// CHECK-NEXT: discriminator = 2
90+
// CHECK-NEXT: case .unavailableMacCatalyst:
91+
// CHECK-NEXT: discriminator = 3
92+
// FIXME: This case should diagnose (rdar://125930716)
93+
// CHECK-NEXT: case .unavailableMacOSAndiOS:
94+
// CHECK-NEXT: discriminator = 4
95+
// FIXME: This case should diagnose (rdar://125930716)
96+
// CHECK-NEXT: case .unavailableMacOSAndMacCatalyst:
97+
// CHECK-NEXT: discriminator = 5
98+
// CHECK-NEXT: }
99+
// CHECK-NEXT: hasher.combine(discriminator)
100+
// CHECK-NEXT: }
101+
102+
// CHECK: internal var hashValue: Int {
103+
// CHECK-NEXT: get {
104+
// CHECK-NEXT: return _hashValue(for: self)
105+
// CHECK-NEXT: }
106+
// CHECK-NEXT: }
107+
}

0 commit comments

Comments
 (0)