Skip to content

Commit c2567ca

Browse files
kavonkavon
authored
Merge pull request #38190 from kavon/5.5-isolated-static-property-silgen
[5.5] Fix SILGen of access to isolated static properties
2 parents ff9c243 + b0ec79c commit c2567ca

File tree

5 files changed

+276
-51
lines changed

5 files changed

+276
-51
lines changed

‎lib/SILGen/LValue.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,18 +207,26 @@ class PathComponent {
207207
/// The only operation on this component is `project`.
208208
class PhysicalPathComponent : public PathComponent {
209209
virtual void _anchor() override;
210+
Optional<ActorIsolation> ActorIso;
210211

211212
protected:
212-
Optional<ActorIsolation> ActorIso;
213213
PhysicalPathComponent(LValueTypeData typeData, KindTy Kind,
214214
Optional<ActorIsolation> actorIso = None)
215215
: PathComponent(typeData, Kind), ActorIso(actorIso) {
216216
assert(isPhysical() && "PhysicalPathComponent Kind isn't physical");
217217
}
218218

219219
public:
220-
// Obtains the actor-isolation required for any loads of this component.
221-
Optional<ActorIsolation> getActorIsolation() const { return ActorIso; }
220+
/// Obtains and consumes the actor-isolation required for any loads of
221+
/// this component.
222+
Optional<ActorIsolation> takeActorIsolation() {
223+
Optional<ActorIsolation> current = ActorIso;
224+
ActorIso = None;
225+
return current;
226+
}
227+
228+
/// Determines whether this component has any actor-isolation.
229+
bool hasActorIsolation() const { return ActorIso.hasValue(); }
222230
};
223231

224232
inline PhysicalPathComponent &PathComponent::asPhysical() {

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -537,9 +537,13 @@ namespace {
537537
};
538538

539539
class EndAccessPseudoComponent : public WritebackPseudoComponent {
540+
private:
541+
ExecutorBreadcrumb ExecutorHop;
540542
public:
541-
EndAccessPseudoComponent(const LValueTypeData &typeData)
542-
: WritebackPseudoComponent(typeData) {}
543+
EndAccessPseudoComponent(const LValueTypeData &typeData,
544+
ExecutorBreadcrumb &&executorHop)
545+
: WritebackPseudoComponent(typeData),
546+
ExecutorHop(std::move(executorHop)) {}
543547

544548
private:
545549
void writeback(SILGenFunction &SGF, SILLocation loc,
@@ -550,6 +554,7 @@ namespace {
550554

551555
assert(base.isLValue());
552556
SGF.B.createEndAccess(loc, base.getValue(), /*abort*/ false);
557+
ExecutorHop.emit(SGF, loc);
553558
}
554559

555560
void dump(raw_ostream &OS, unsigned indent) const override {
@@ -561,13 +566,19 @@ namespace {
561566
static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
562567
SILValue addr, LValueTypeData typeData,
563568
SGFAccessKind accessKind,
564-
SILAccessEnforcement enforcement) {
569+
SILAccessEnforcement enforcement,
570+
Optional<ActorIsolation> actorIso) {
565571
auto silAccessKind = isReadAccess(accessKind) ? SILAccessKind::Read
566572
: SILAccessKind::Modify;
567573

568574
assert(SGF.isInFormalEvaluationScope() &&
569575
"tried to enter access scope without a writeback scope!");
570576

577+
// Only expecting global-actor isolation here, since there's no base / self.
578+
assert(!actorIso || actorIso->isGlobalActor());
579+
ExecutorBreadcrumb prevExecutor =
580+
SGF.emitHopToTargetActor(loc, actorIso, /*maybeSelf=*/None);
581+
571582
// Enter the access.
572583
addr = SGF.B.createBeginAccess(loc, addr, silAccessKind, enforcement,
573584
/*hasNoNestedConflict=*/false,
@@ -576,7 +587,8 @@ static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
576587
// Push a writeback to end it.
577588
auto accessedMV = ManagedValue::forLValue(addr);
578589
std::unique_ptr<LogicalPathComponent>
579-
component(new EndAccessPseudoComponent(typeData));
590+
component(new EndAccessPseudoComponent(typeData,
591+
std::move(prevExecutor)));
580592
pushWriteback(SGF, loc, std::move(component), accessedMV,
581593
MaterializedLValue());
582594

@@ -586,10 +598,11 @@ static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
586598
static ManagedValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
587599
ManagedValue addr, LValueTypeData typeData,
588600
SGFAccessKind accessKind,
589-
SILAccessEnforcement enforcement) {
601+
SILAccessEnforcement enforcement,
602+
Optional<ActorIsolation> actorIso) {
590603
return ManagedValue::forLValue(
591604
enterAccessScope(SGF, loc, addr.getLValueAddress(), typeData,
592-
accessKind, enforcement));
605+
accessKind, enforcement, actorIso));
593606
}
594607

595608
// Find the base of the formal access at `address`. If the base requires an
@@ -717,8 +730,9 @@ namespace {
717730
bool IsNonAccessing;
718731
public:
719732
RefElementComponent(VarDecl *field, LValueOptions options,
720-
SILType substFieldType, LValueTypeData typeData)
721-
: PhysicalPathComponent(typeData, RefElementKind),
733+
SILType substFieldType, LValueTypeData typeData,
734+
Optional<ActorIsolation> actorIso)
735+
: PhysicalPathComponent(typeData, RefElementKind, actorIso),
722736
Field(field), SubstFieldType(substFieldType),
723737
IsNonAccessing(options.IsNonAccessing) {}
724738

@@ -745,7 +759,8 @@ namespace {
745759
if (!IsNonAccessing && !Field->isLet()) {
746760
if (auto enforcement = SGF.getDynamicEnforcement(Field)) {
747761
result = enterAccessScope(SGF, loc, result, getTypeData(),
748-
getAccessKind(), *enforcement);
762+
getAccessKind(), *enforcement,
763+
takeActorIsolation());
749764
}
750765
}
751766

@@ -761,7 +776,8 @@ namespace {
761776
unsigned ElementIndex;
762777
public:
763778
TupleElementComponent(unsigned elementIndex, LValueTypeData typeData)
764-
: PhysicalPathComponent(typeData, TupleElementKind),
779+
: PhysicalPathComponent(typeData, TupleElementKind,
780+
/*actorIsolation=*/None),
765781
ElementIndex(elementIndex) {}
766782

767783
virtual bool isLoadingPure() const override { return true; }
@@ -790,8 +806,9 @@ namespace {
790806
SILType SubstFieldType;
791807
public:
792808
StructElementComponent(VarDecl *field, SILType substFieldType,
793-
LValueTypeData typeData)
794-
: PhysicalPathComponent(typeData, StructElementKind),
809+
LValueTypeData typeData,
810+
Optional<ActorIsolation> actorIso)
811+
: PhysicalPathComponent(typeData, StructElementKind, actorIso),
795812
Field(field), SubstFieldType(substFieldType) {}
796813

797814
virtual bool isLoadingPure() const override { return true; }
@@ -819,9 +836,9 @@ namespace {
819836
class ForceOptionalObjectComponent : public PhysicalPathComponent {
820837
bool isImplicitUnwrap;
821838
public:
822-
ForceOptionalObjectComponent(LValueTypeData typeData,
823-
bool isImplicitUnwrap)
824-
: PhysicalPathComponent(typeData, OptionalObjectKind),
839+
ForceOptionalObjectComponent(LValueTypeData typeData, bool isImplicitUnwrap)
840+
: PhysicalPathComponent(typeData, OptionalObjectKind,
841+
/*actorIsolation=*/None),
825842
isImplicitUnwrap(isImplicitUnwrap) {}
826843

827844
ManagedValue project(SILGenFunction &SGF, SILLocation loc,
@@ -842,7 +859,8 @@ namespace {
842859
public:
843860
OpenOpaqueExistentialComponent(CanArchetypeType openedArchetype,
844861
LValueTypeData typeData)
845-
: PhysicalPathComponent(typeData, OpenOpaqueExistentialKind) {
862+
: PhysicalPathComponent(typeData, OpenOpaqueExistentialKind,
863+
/*actorIsolation=*/None) {
846864
assert(getSubstFormalType() == openedArchetype);
847865
}
848866

@@ -1017,7 +1035,8 @@ namespace {
10171035

10181036
SILValue addr = Value.getLValueAddress();
10191037
addr = enterAccessScope(SGF, loc, addr, getTypeData(),
1020-
getAccessKind(), *Enforcement);
1038+
getAccessKind(), *Enforcement,
1039+
takeActorIsolation());
10211040

10221041
return ManagedValue::forLValue(addr);
10231042
}
@@ -1034,10 +1053,7 @@ namespace {
10341053
} else {
10351054
OS << "unenforced";
10361055
}
1037-
if (ActorIso) {
1038-
OS << "requires actor-hop because ";
1039-
simple_display(OS, *ActorIso);
1040-
}
1056+
if (hasActorIsolation()) OS << " requires actor-hop";
10411057
OS << "):\n";
10421058
Value.dump(OS, indent + 2);
10431059
}
@@ -1451,11 +1467,12 @@ namespace {
14511467
backingVar, AccessKind::Write);
14521468
} else if (BaseFormalType->mayHaveSuperclass()) {
14531469
RefElementComponent REC(backingVar, LValueOptions(), varStorageType,
1454-
typeData);
1470+
typeData, /*actorIsolation=*/None);
14551471
proj = std::move(REC).project(SGF, loc, base);
14561472
} else {
14571473
assert(BaseFormalType->getStructOrBoundGenericStruct());
1458-
StructElementComponent SEC(backingVar, varStorageType, typeData);
1474+
StructElementComponent SEC(backingVar, varStorageType,
1475+
typeData, /*actorIsolation=*/None);
14591476
proj = std::move(SEC).project(SGF, loc, base);
14601477
}
14611478

@@ -1782,7 +1799,8 @@ namespace {
17821799

17831800
// Enter an unsafe access scope for the access.
17841801
addr = enterAccessScope(SGF, loc, addr, getTypeData(), getAccessKind(),
1785-
SILAccessEnforcement::Unsafe);
1802+
SILAccessEnforcement::Unsafe,
1803+
ActorIso);
17861804

17871805
return addr;
17881806
}
@@ -2087,7 +2105,8 @@ namespace {
20872105
PhysicalKeyPathApplicationComponent(LValueTypeData typeData,
20882106
KeyPathTypeKind typeKind,
20892107
ManagedValue keyPath)
2090-
: PhysicalPathComponent(typeData, PhysicalKeyPathApplicationKind),
2108+
: PhysicalPathComponent(typeData, PhysicalKeyPathApplicationKind,
2109+
/*actorIsolation=*/None),
20912110
TypeKind(typeKind), KeyPath(keyPath) {
20922111
assert(typeKind == KPTK_KeyPath ||
20932112
typeKind == KPTK_WritableKeyPath ||
@@ -3274,8 +3293,6 @@ void LValue::addMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
32743293
using MemberStorageAccessEmitter::MemberStorageAccessEmitter;
32753294

32763295
void emitUsingStorage(LValueTypeData typeData) {
3277-
assert(!ActorIso);
3278-
32793296
// For static variables, emit a reference to the global variable backing
32803297
// them.
32813298
// FIXME: This has to be dynamically looked up for classes, and
@@ -3289,7 +3306,7 @@ void LValue::addMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
32893306
typeData.getAccessKind(),
32903307
AccessStrategy::getStorage(),
32913308
FormalRValueType,
3292-
/*actorIsolation=*/None);
3309+
ActorIso);
32933310
return;
32943311
}
32953312

@@ -3298,10 +3315,12 @@ void LValue::addMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
32983315
SGF.getTypeExpansionContext(), Storage, FormalRValueType);
32993316

33003317
if (BaseFormalType->mayHaveSuperclass()) {
3301-
LV.add<RefElementComponent>(Storage, Options, varStorageType, typeData);
3318+
LV.add<RefElementComponent>(Storage, Options, varStorageType,
3319+
typeData, ActorIso);
33023320
} else {
33033321
assert(BaseFormalType->getStructOrBoundGenericStruct());
3304-
LV.add<StructElementComponent>(Storage, varStorageType, typeData);
3322+
LV.add<StructElementComponent>(Storage, varStorageType, typeData,
3323+
ActorIso);
33053324
}
33063325

33073326
// If the member has weak or unowned storage, convert it away.
@@ -4174,18 +4193,17 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
41744193

41754194
// If the last component is physical, drill down and load from it.
41764195
if (component.isPhysical()) {
4177-
auto actorIso = component.asPhysical().getActorIsolation();
4178-
4179-
// If the load must happen in the context of an actor, do a hop first.
4180-
prevExecutor = emitHopToTargetActor(loc, actorIso, /*actorSelf=*/None);
4181-
41824196
auto projection = std::move(component).project(*this, loc, addr);
41834197
if (projection.getType().isAddress()) {
4198+
auto actorIso = component.asPhysical().takeActorIsolation();
4199+
4200+
// If the load must happen in the context of an actor, do a hop first.
4201+
prevExecutor = emitHopToTargetActor(loc, actorIso, /*actorSelf=*/None);
41844202
projection =
41854203
emitLoad(loc, projection.getValue(), origFormalType,
41864204
substFormalType, rvalueTL, C, IsNotTake, isBaseGuaranteed);
41874205
} else if (isReadAccessResultOwned(src.getAccessKind()) &&
4188-
!projection.isPlusOne(*this)) {
4206+
!projection.isPlusOne(*this)) {
41894207
projection = projection.copy(*this, loc);
41904208
}
41914209

@@ -4198,7 +4216,6 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
41984216

41994217
// If we hopped to the target's executor, then we need to hop back.
42004218
prevExecutor.emit(*this, loc);
4201-
42024219
return result;
42034220
}
42044221

‎test/Concurrency/Runtime/async_properties_actor.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,21 @@
88
// UNSUPPORTED: use_os_stdlib
99
// UNSUPPORTED: back_deployment_runtime
1010

11+
struct Container {
12+
@MainActor static var counter: Int = 10
13+
@MainActor static var this: Container?
14+
15+
var noniso: Int = 20
16+
17+
static func getCount() async -> Int {
18+
return await counter
19+
}
20+
21+
static func getValue() async -> Int? {
22+
return await this?.noniso
23+
}
24+
}
25+
1126
@propertyWrapper
1227
struct SuccessTracker {
1328
private var _stored: Bool
@@ -142,6 +157,13 @@ actor Tester {
142157
print("done property wrapper test")
143158
return true
144159
}
160+
161+
func doContainerTest() async -> Bool {
162+
var total: Int = 0
163+
total += await Container.getCount()
164+
total += await Container.getValue() ?? 0
165+
return total == 10
166+
}
145167
}
146168

147169
@globalActor
@@ -171,6 +193,10 @@ var expectedResult : Bool {
171193
fatalError("fail property wrapper test")
172194
}
173195

196+
guard await test.doContainerTest() == success else {
197+
fatalError("fail container test")
198+
}
199+
174200
// CHECK: done all testing
175201
print("done all testing")
176202
}

0 commit comments

Comments
 (0)