Skip to content

Commit c1e79d8

Browse files
committed
fix SILGen of access to isolated wrapper around instance actor property
The fix is to implement part of SILGen that I asserted could not happen because I couldn't formulate a situation where an access to instance-isolated state would happen at the point in the code. But, if a property wrapper's wrapped-value is global-actor isolated, and is wrapping an actor instance, then it does happen. Implementation details: If the address is isolated to an actor instance, then we need the base in order to perform a hop prior to starting the access, so I just pass the base value through SILGen to the point it is needed, since the base is available from all of the callers.
1 parent a8c2aaa commit c1e79d8

File tree

2 files changed

+54
-14
lines changed

2 files changed

+54
-14
lines changed

lib/SILGen/SILGenLValue.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,8 @@ namespace {
564564
} // end anonymous namespace
565565

566566
static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
567-
SILValue addr, LValueTypeData typeData,
567+
ManagedValue base, SILValue addr,
568+
LValueTypeData typeData,
568569
SGFAccessKind accessKind,
569570
SILAccessEnforcement enforcement,
570571
Optional<ActorIsolation> actorIso) {
@@ -574,10 +575,8 @@ static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
574575
assert(SGF.isInFormalEvaluationScope() &&
575576
"tried to enter access scope without a writeback scope!");
576577

577-
// Only expecting global-actor isolation here, since there's no base / self.
578-
assert(!actorIso || actorIso->isGlobalActor());
579578
ExecutorBreadcrumb prevExecutor =
580-
SGF.emitHopToTargetActor(loc, actorIso, /*maybeSelf=*/None);
579+
SGF.emitHopToTargetActor(loc, actorIso, base);
581580

582581
// Enter the access.
583582
addr = SGF.B.createBeginAccess(loc, addr, silAccessKind, enforcement,
@@ -596,13 +595,14 @@ static SILValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
596595
}
597596

598597
static ManagedValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
599-
ManagedValue addr, LValueTypeData typeData,
598+
ManagedValue base, ManagedValue addr,
599+
LValueTypeData typeData,
600600
SGFAccessKind accessKind,
601601
SILAccessEnforcement enforcement,
602602
Optional<ActorIsolation> actorIso) {
603603
return ManagedValue::forLValue(
604-
enterAccessScope(SGF, loc, addr.getLValueAddress(), typeData,
605-
accessKind, enforcement, actorIso));
604+
enterAccessScope(SGF, loc, base, addr.getLValueAddress(), typeData,
605+
accessKind, enforcement, actorIso));
606606
}
607607

608608
// Find the base of the formal access at `address`. If the base requires an
@@ -758,7 +758,7 @@ namespace {
758758
// declarations. Access marker verification is aware of these cases.
759759
if (!IsNonAccessing && !Field->isLet()) {
760760
if (auto enforcement = SGF.getDynamicEnforcement(Field)) {
761-
result = enterAccessScope(SGF, loc, result, getTypeData(),
761+
result = enterAccessScope(SGF, loc, base, result, getTypeData(),
762762
getAccessKind(), *enforcement,
763763
takeActorIsolation());
764764
}
@@ -1034,9 +1034,9 @@ namespace {
10341034
return Value;
10351035

10361036
SILValue addr = Value.getLValueAddress();
1037-
addr = enterAccessScope(SGF, loc, addr, getTypeData(),
1038-
getAccessKind(), *Enforcement,
1039-
takeActorIsolation());
1037+
addr =
1038+
enterAccessScope(SGF, loc, base, addr, getTypeData(), getAccessKind(),
1039+
*Enforcement, takeActorIsolation());
10401040

10411041
return ManagedValue::forLValue(addr);
10421042
}
@@ -1798,9 +1798,9 @@ namespace {
17981798
}
17991799

18001800
// Enter an unsafe access scope for the access.
1801-
addr = enterAccessScope(SGF, loc, addr, getTypeData(), getAccessKind(),
1802-
SILAccessEnforcement::Unsafe,
1803-
ActorIso);
1801+
addr =
1802+
enterAccessScope(SGF, loc, base, addr, getTypeData(), getAccessKind(),
1803+
SILAccessEnforcement::Unsafe, ActorIso);
18041804

18051805
return addr;
18061806
}

test/SILGen/hop_to_executor_async_prop.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,4 +603,44 @@ struct Container {
603603
static func getRefOrCrash() async -> CatBox {
604604
return await this!.isoRef
605605
}
606+
}
607+
608+
609+
@propertyWrapper
610+
struct StateObject<ObjectType> {
611+
@MainActor(unsafe)
612+
var wrappedValue: ObjectType {
613+
fatalError()
614+
}
615+
init(wrappedValue: ObjectType) {}
616+
}
617+
618+
final private actor Coordinactor {
619+
var someValue: Int?
620+
}
621+
622+
struct Blah {
623+
@StateObject private var coordinator = Coordinactor()
624+
625+
// closure #1 in Blah.test()
626+
// CHECK-LABEL: sil private [ossa] @$s4test4BlahVAAyyFyyYaYbcfU_ : $@convention(thin) @Sendable @async (Blah) -> () {
627+
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
628+
// CHECK: [[ACTOR_OBJ_RAW:%[0-9]+]] = apply {{%[0-9]+}}({{%[0-9]+}}) : $@convention(method) (Blah) -> @owned Coordinactor
629+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
630+
// CHECK: [[ACTOR_OBJ:%[0-9]+]] = begin_borrow [[ACTOR_OBJ_RAW]] : $Coordinactor
631+
// CHECK: [[VAL:%[0-9]+]] = ref_element_addr [[ACTOR_OBJ]] : $Coordinactor, #Coordinactor.someValue
632+
// CHECK: hop_to_executor [[ACTOR_OBJ]]
633+
// CHECK: [[VAL_ACCESS:%[0-9]+]] = begin_access [read] [dynamic] [[VAL]] : $*Optional<Int>
634+
// CHECK: {{%[0-9]+}} = load [trivial] %17 : $*Optional<Int>
635+
// CHECK: end_access %17 : $*Optional<Int>
636+
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
637+
// CHECK: } // end sil function '$s4test4BlahVAAyyFyyYaYbcfU_'
638+
@available(SwiftStdlib 5.5, *)
639+
func test() {
640+
Task.detached {
641+
if await coordinator.someValue == nil {
642+
fatalError()
643+
}
644+
}
645+
}
606646
}

0 commit comments

Comments
 (0)