Skip to content

Commit 10ee948

Browse files
nickolas-pohiletsnickolas-pohilets
committed
Generating errors when isolation of the subclass deinit is not compatible with superclass deinit
1 parent d4e449b commit 10ee948

File tree

5 files changed

+239
-7
lines changed

5 files changed

+239
-7
lines changed

‎include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3031,6 +3031,10 @@ class ValueDecl : public Decl {
30313031
/// Retrieve the declaration that this declaration overrides, if any.
30323032
ValueDecl *getOverriddenDecl() const;
30333033

3034+
/// Retrieve the declaration that this declaration overrides, including super
3035+
/// deinit.
3036+
ValueDecl *getOverriddenDeclOrSuperDeinit() const;
3037+
30343038
/// Retrieve the declarations that this declaration overrides, if any.
30353039
llvm::TinyPtrVector<ValueDecl *> getOverriddenDecls() const;
30363040

‎lib/AST/Decl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3269,6 +3269,20 @@ ValueDecl *ValueDecl::getOverriddenDecl() const {
32693269
return overridden.front();
32703270
}
32713271

3272+
ValueDecl *ValueDecl::getOverriddenDeclOrSuperDeinit() const {
3273+
if (auto overridden = getOverriddenDecl()) {
3274+
return overridden;
3275+
}
3276+
if (isa<DestructorDecl>(this)) {
3277+
if (auto classDecl = dyn_cast<ClassDecl>(getDeclContext())) {
3278+
if (auto superclass = classDecl->getSuperclassDecl()) {
3279+
return superclass->getDestructor();
3280+
}
3281+
}
3282+
}
3283+
return nullptr;
3284+
}
3285+
32723286
bool ValueDecl::overriddenDeclsComputed() const {
32733287
return LazySemanticInfo.hasOverriddenComputed;
32743288
}

‎lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4917,7 +4917,7 @@ namespace {
49174917
/// Return the isolation of the declaration overridden by this declaration,
49184918
/// in the context of the
49194919
static ActorIsolation getOverriddenIsolationFor(ValueDecl *value) {
4920-
auto overridden = value->getOverriddenDecl();
4920+
auto overridden = value->getOverriddenDeclOrSuperDeinit();
49214921
assert(overridden && "Doesn't have an overridden declaration");
49224922

49234923
auto isolation = getActorIsolation(overridden);
@@ -4953,9 +4953,16 @@ static OverrideIsolationResult validOverrideIsolation(
49534953
auto declContext = value->getInnermostDeclContext();
49544954
auto &ctx = declContext->getASTContext();
49554955

4956+
// Normally we are checking if overriding declaration can be called by calling
4957+
// overriden declaration But in case of destructors, overriden declaration is
4958+
// always callable by definition and we are checking that subclas deinit can
4959+
// call super deinit.
4960+
bool isDtor = isa<DestructorDecl>(value);
4961+
49564962
auto refResult = ActorReferenceResult::forReference(
4957-
valueRef, SourceLoc(), declContext, std::nullopt, std::nullopt, isolation,
4958-
overriddenIsolation);
4963+
valueRef, SourceLoc(), declContext, std::nullopt, std::nullopt,
4964+
isDtor ? overriddenIsolation : isolation,
4965+
isDtor ? isolation : overriddenIsolation);
49594966
switch (refResult) {
49604967
case ActorReferenceResult::SameConcurrencyDomain:
49614968
return OverrideIsolationResult::Allowed;
@@ -5558,7 +5565,7 @@ void swift::checkOverrideActorIsolation(ValueDecl *value) {
55585565
if (isa<TypeDecl>(value))
55595566
return;
55605567

5561-
auto overridden = value->getOverriddenDecl();
5568+
auto overridden = value->getOverriddenDeclOrSuperDeinit();
55625569
if (!overridden)
55635570
return;
55645571

@@ -6683,12 +6690,12 @@ static bool isNonValueReference(const ValueDecl *value) {
66836690
case DeclKind::PrecedenceGroup:
66846691
case DeclKind::PrefixOperator:
66856692
case DeclKind::TopLevelCode:
6686-
case DeclKind::Destructor:
66876693
case DeclKind::MacroExpansion:
66886694
return true;
66896695

66906696
case DeclKind::EnumElement:
66916697
case DeclKind::Constructor:
6698+
case DeclKind::Destructor:
66926699
case DeclKind::Param:
66936700
case DeclKind::Var:
66946701
case DeclKind::Accessor:

‎lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2300,8 +2300,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
23002300
(void) VD->getFormalAccess();
23012301

23022302
// Compute overrides.
2303-
if (!VD->getOverriddenDecls().empty())
2304-
checkOverrideActorIsolation(VD);
2303+
checkOverrideActorIsolation(VD);
23052304

23062305
// Check whether the member is @objc or dynamic.
23072306
(void) VD->isObjC();

‎test/Concurrency/deinit_isolation.swift

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// RUN: %target-swift-frontend -parse-as-library -emit-sil -verify %s
2+
3+
// Fixtures
4+
5+
@globalActor final actor FirstActor {
6+
static let shared: FirstActor = FirstActor()
7+
}
8+
9+
@globalActor final actor SecondActor {
10+
static let shared: SecondActor = SecondActor()
11+
}
12+
13+
14+
@FirstActor
15+
func isolatedFunc() {} // expected-note 10{{calls to global function 'isolatedFunc()' from outside of its actor context are implicitly asynchronous}}
16+
17+
class BaseWithNonisolatedDeinit {}
18+
19+
class BaseWithDeinitIsolatedOnFirstActor {
20+
@FirstActor deinit {}
21+
}
22+
23+
class BaseWithDeinitIsolatedOnSecondActor {
24+
@SecondActor deinit {}
25+
}
26+
27+
//
28+
// Part 1 - Actors
29+
//
30+
31+
actor ImplicitDeinitActor {
32+
// nonisolated deinit
33+
}
34+
35+
actor ExplicitDeinitActor {
36+
// self-isolated deinit
37+
deinit {
38+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous actor-isolated context}}
39+
}
40+
}
41+
42+
actor NonisolatedDeinitActor {
43+
// nonisolated deinit
44+
nonisolated deinit {
45+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous nonisolated context}}
46+
}
47+
}
48+
49+
actor IsolatedDeinitActor {
50+
// FirstActor-isolated deinit
51+
@FirstActor deinit {
52+
isolatedFunc() // ok
53+
}
54+
}
55+
56+
// Part 2 - Classes
57+
// Part 2.1 - Without base class
58+
59+
@FirstActor
60+
class ImplicitDeinit {
61+
// nonisolated deinit
62+
}
63+
64+
@FirstActor
65+
class ExplicitDeinit {
66+
// FirstActor-isolated deinit
67+
deinit {
68+
isolatedFunc() // ok
69+
}
70+
}
71+
72+
@FirstActor
73+
class NonisolatedDeinit {
74+
// nonisolated deinit
75+
nonisolated deinit {
76+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous nonisolated context}}
77+
}
78+
}
79+
80+
class IsolatedDeinit {
81+
// FirstActor-isolated deinit
82+
@FirstActor deinit {
83+
isolatedFunc() // ok
84+
}
85+
}
86+
87+
@FirstActor
88+
class DifferentIsolatedDeinit {
89+
// SecondActor-isolated deinit
90+
@SecondActor deinit {
91+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous global actor 'SecondActor'-isolated context}}
92+
}
93+
}
94+
95+
// Part 2.2 - Base class with nonisolated deinit
96+
97+
@FirstActor
98+
class ImplicitDeinitInheritNonisolated: BaseWithNonisolatedDeinit {
99+
// nonisolated deinit
100+
}
101+
102+
@FirstActor
103+
class ExplicitDeinitInheritNonisolated: BaseWithNonisolatedDeinit {
104+
// FirstActor-isolated deinit
105+
deinit {
106+
isolatedFunc() // ok
107+
}
108+
}
109+
110+
@FirstActor
111+
class NonisolatedDeinitInheritNonisolated: BaseWithNonisolatedDeinit {
112+
// nonisolated deinit
113+
nonisolated deinit {
114+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous nonisolated context}}
115+
}
116+
}
117+
118+
class IsolatedDeinitInheritNonisolated: BaseWithNonisolatedDeinit {
119+
// FirstActor-isolated deinit
120+
@FirstActor deinit {
121+
isolatedFunc() // ok
122+
}
123+
}
124+
125+
@FirstActor
126+
class DifferentIsolatedDeinitInheritNonisolated: BaseWithNonisolatedDeinit {
127+
// SecondActor-isolated deinit
128+
@SecondActor deinit {
129+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous global actor 'SecondActor'-isolated context}}
130+
}
131+
}
132+
133+
// Part 2.3 - Base class with isolated deinit
134+
135+
@FirstActor
136+
class ImplicitDeinitInheritIsolated1: BaseWithDeinitIsolatedOnFirstActor {
137+
// FirstActor-isolated deinit
138+
}
139+
140+
@FirstActor
141+
class ExplicitDeinitIsolated1: BaseWithDeinitIsolatedOnFirstActor {
142+
// FirstActor-isolated deinit
143+
deinit {
144+
isolatedFunc() // ok
145+
}
146+
}
147+
148+
@FirstActor
149+
class NonisolatedDeinitIsolated1: BaseWithDeinitIsolatedOnFirstActor {
150+
// FirstActor-isolated deinit
151+
nonisolated deinit { // expected-error {{nonisolated deinitializer 'deinit' has different actor isolation from global actor 'FirstActor'-isolated overridden declaration}}
152+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous nonisolated context}}
153+
}
154+
}
155+
156+
class IsolatedDeinitIsolated1: BaseWithDeinitIsolatedOnFirstActor {
157+
// FirstActor-isolated deinit
158+
@FirstActor deinit {
159+
isolatedFunc() // pl
160+
}
161+
}
162+
163+
@FirstActor
164+
class DifferentIsolatedDeinitIsolated1: BaseWithDeinitIsolatedOnFirstActor {
165+
// error
166+
@SecondActor deinit { // expected-error {{global actor 'SecondActor'-isolated deinitializer 'deinit' has different actor isolation from global actor 'FirstActor'-isolated overridden declaration}}
167+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous global actor 'SecondActor'-isolated context}}
168+
}
169+
}
170+
171+
// Part 2.4 - Base class with isolated deinit with different actor
172+
173+
@FirstActor
174+
class ImplicitDeinitInheritIsolated2: BaseWithDeinitIsolatedOnSecondActor {
175+
// SecondActor-isolated deinit
176+
}
177+
178+
@FirstActor
179+
class ExplicitDeinitIsolated2: BaseWithDeinitIsolatedOnSecondActor {
180+
// SecondActor-isolated deinit
181+
deinit {
182+
isolatedFunc() // ok
183+
}
184+
}
185+
186+
@FirstActor
187+
class NonisolatedDeinitIsolated2: BaseWithDeinitIsolatedOnSecondActor {
188+
// SecondActor-isolated deinit
189+
nonisolated deinit {
190+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous nonisolated context}}
191+
}
192+
}
193+
194+
class IsolatedDeinitIsolated2: BaseWithDeinitIsolatedOnSecondActor {
195+
// // error
196+
@FirstActor deinit {
197+
isolatedFunc() // error
198+
}
199+
}
200+
201+
@FirstActor
202+
class DifferentIsolatedDeinitIsolated2: BaseWithDeinitIsolatedOnSecondActor {
203+
// SecondActor-isolated deinit
204+
@SecondActor deinit {
205+
isolatedFunc() // expected-error {{call to global actor 'FirstActor'-isolated global function 'isolatedFunc()' in a synchronous global actor 'SecondActor'-isolated context}}
206+
}
207+
}
208+

0 commit comments

Comments
 (0)