Skip to content

Fixes for stored property initializers #40652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8682,10 +8682,23 @@ ActorIsolation swift::getActorIsolationOfContext(DeclContext *dc) {
if (auto *vd = dyn_cast_or_null<ValueDecl>(dc->getAsDecl()))
return getActorIsolation(vd);

// In the context of the initializing or default-value expression of a
// stored property, the isolation varies between global and type members:
// - For a static stored property, the isolation matches the VarDecl.
// - For a field of a nominal type, the expression is not isolated.
// Without this distinction, a nominal can have non-async initializers
// with various kinds of isolation, so an impossible constraint can be
// created. See SE-0327 for details.
if (auto *init = dyn_cast<PatternBindingInitializer>(dc)) {
if (auto *var = init->getBinding()->getAnchoringVarDecl(
init->getBindingIndex()))
if (auto *var =
init->getBinding()->getAnchoringVarDecl(init->getBindingIndex())) {

if (var->isInstanceMember() &&
!var->getAttrs().hasAttribute<LazyAttr>())
return ActorIsolation::forUnspecified();

return getActorIsolation(var);
}
}

if (auto *closure = dyn_cast<AbstractClosureExpr>(dc)) {
Expand Down
4 changes: 3 additions & 1 deletion lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
/// Generate an ObjC-compatible destructor (-dealloc).
void emitObjCDestructor(SILDeclRef dtor);

ManagedValue emitGlobalVariableRef(SILLocation loc, VarDecl *var);
/// Generate code to obtain the address of the given global variable.
ManagedValue emitGlobalVariableRef(SILLocation loc, VarDecl *var,
Optional<ActorIsolation> actorIso);

/// Generate a lazy global initializer.
void emitLazyGlobalInitializer(PatternBindingDecl *binding,
Expand Down
18 changes: 17 additions & 1 deletion lib/SILGen/SILGenGlobalVariable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

#include "SILGenFunction.h"
#include "ExecutorBreadcrumb.h"
#include "ManagedValue.h"
#include "Scope.h"
#include "swift/AST/ASTMangler.h"
Expand Down Expand Up @@ -66,7 +67,8 @@ SILGlobalVariable *SILGenModule::getSILGlobalVariable(VarDecl *gDecl,
}

ManagedValue
SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var) {
SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var,
Optional<ActorIsolation> actorIso) {
assert(!VarLocs.count(var));

if (var->isLazilyInitializedGlobal()) {
Expand All @@ -75,7 +77,21 @@ SILGenFunction::emitGlobalVariableRef(SILLocation loc, VarDecl *var) {
SILDeclRef(var, SILDeclRef::Kind::GlobalAccessor),
NotForDefinition);
SILValue accessor = B.createFunctionRefFor(loc, accessorFn);

// The accessor to obtain a global's address may need to initialize the
// variable first. So, we must call this accessor with the same
// isolation that the variable itself requires during access.
ExecutorBreadcrumb prevExecutor = emitHopToTargetActor(loc, actorIso,
/*base=*/None);

SILValue addr = B.createApply(loc, accessor, SubstitutionMap(), {});

// FIXME: often right after this, we will again hop to the actor to
// read from this address. it would be better to merge these two hops
// pairs of hops together. Alternatively, teaching optimizations to
// expand the scope of two nearly-adjacent pairs would be good.
prevExecutor.emit(*this, loc); // hop back after call.

// FIXME: It'd be nice if the result of the accessor was natively an
// address.
addr = B.createPointerToAddress(
Expand Down
2 changes: 1 addition & 1 deletion lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2813,7 +2813,7 @@ void LValue::addNonMemberVarComponent(SILGenFunction &SGF, SILLocation loc,

// The only other case that should get here is a global variable.
if (!address) {
address = SGF.emitGlobalVariableRef(Loc, Storage);
address = SGF.emitGlobalVariableRef(Loc, Storage, ActorIso);
} else {
assert(!ActorIso && "local var should not be actor isolated!");
}
Expand Down
26 changes: 26 additions & 0 deletions test/Concurrency/Runtime/mainactor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ actor A {
return await enterMainActor(0) + 1
}

@MainActor func mainActorFn() -> Int {
checkIfMainQueue(expectedAnswer: true)
return 10
}

@MainActor
struct S {
static var bacteria: Int = mainActorFn()
}

@MainActor
class C {
static var bacteria: Int = mainActorFn()
lazy var amoeba: Int = mainActorFn()
nonisolated init() {}
}

// CHECK: starting
// CHECK-NOT: ERROR
Expand Down Expand Up @@ -115,5 +131,15 @@ actor A {
}
}
_ = await task.value

// Check that initializers for stored properties are on the right actor
let t1 = Task.detached { () -> Int in
let c = C()
return await c.amoeba
}
let t2 = Task.detached { () -> Int in
return await S.bacteria + C.bacteria
}
_ = await t1.value + t2.value
}
}
18 changes: 18 additions & 0 deletions test/Concurrency/property_initializers.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking -warn-concurrency
// REQUIRES: concurrency


@MainActor
func mainActorFn() -> Int { return 0 } // expected-note 2 {{calls to global function 'mainActorFn()' from outside of its actor context are implicitly asynchronous}}

@MainActor
class C {
var x: Int = mainActorFn() // expected-error {{call to main actor-isolated global function 'mainActorFn()' in a synchronous nonisolated context}}

lazy var y: Int = mainActorFn()

static var z: Int = mainActorFn()
}

@MainActor
var x: Int = mainActorFn() // expected-error {{call to main actor-isolated global function 'mainActorFn()' in a synchronous nonisolated context}}
61 changes: 54 additions & 7 deletions test/SILGen/hop_to_executor_async_prop.swift
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ struct Container {
}

// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV12accessTuple3SfyYaF : $@convention(method) @async (@guaranteed Container) -> Float {
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV12staticCircleSi_SitSg_Sftvau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*(Optional<(Int, Int)>, Float)
// CHECK: [[ADDR:%[0-9]+]] = tuple_element_addr [[ACCESS]] : $*(Optional<(Int, Int)>, Float), 1
Expand All @@ -500,6 +504,10 @@ struct Container {
}

// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV12accessTuple4SiSgyYaFZ : $@convention(method) @async (@thin Container.Type) -> Optional<Int> {
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV12staticCircleSi_SitSg_Sftvau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*(Optional<(Int, Int)>, Float)
// CHECK: [[ADDR:%[0-9]+]] = tuple_element_addr [[ACCESS]] : $*(Optional<(Int, Int)>, Float), 0
Expand All @@ -522,6 +530,10 @@ struct Container {


// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV8getCountSiyYaFZ : $@convention(method) @async (@thin Container.Type) -> Int {
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV7counterSivau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: {{%[0-9]+}} = begin_access [read] [dynamic] {{%[0-9]+}} : $*Int
// CHECK: {{%[0-9]+}} = load [trivial] {{%[0-9]+}} : $*Int
Expand All @@ -534,6 +546,10 @@ struct Container {

// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV8getValueSiSgyYaFZ : $@convention(method) @async (@thin Container.Type) -> Optional<Int> {
// CHECK: bb0(%0 : $@thin Container.Type):
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[MAIN]] : $MainActor
Expand All @@ -556,19 +572,23 @@ struct Container {

// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV10getOrCrashSfyYaFZ : $@convention(method) @async (@thin Container.Type) -> Float {
// CHECK: bb0({{%[0-9]+}} : $@thin Container.Type):
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[MAIN]] : $MainActor
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*Optional<Container>
// CHECK: switch_enum_addr %11 : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
// CHECK: switch_enum_addr [[ACCESS]] : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
//
// CHECK: [[CRASH_BB]]:
// CHECK-NOT: hop_to_executor {{%[0-9]+}}
// CHECK: unreachable
//
// CHECK: [[SOME_BB]]:
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr %11 : $*Optional<Container>, #Optional.some!enumelt
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr %22 : $*Container, #Container.iso
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
// CHECK: [[PREV_AGAIN:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
// CHECK: {{%[0-9]+}} = load [trivial] [[ELEM_ADDR]] : $*Float
Expand All @@ -581,19 +601,22 @@ struct Container {

// CHECK-LABEL: sil hidden [ossa] @$s4test9ContainerV13getRefOrCrashAA6CatBoxCyYaFZ : $@convention(method) @async (@thin Container.Type) -> @owned CatBox {
// CHECK: bb0({{%[0-9]+}} : $@thin Container.Type):
// CHECK: [[ADDRESS_ACCESSOR:%[0-9]+]] = function_ref @$s4test9ContainerV4thisACSgvau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $MainActor
// CHECK: = apply [[ADDRESS_ACCESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%[0-9]+}} : $Optional<Builtin.Executor>
// CHECK: [[MAIN:%[0-9]+]] = begin_borrow {{%[0-9]+}} : $MainActor
// CHECK: [[PREV:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
// CHECK: hop_to_executor [[MAIN]] : $MainActor
// CHECK: [[ACCESS:%[0-9]+]] = begin_access [read] [dynamic] {{%[0-9]+}} : $*Optional<Container>
// CHECK: switch_enum_addr %11 : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
// CHECK: switch_enum_addr [[ACCESS]] : $*Optional<Container>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]+]], case #Optional.none!enumelt: [[CRASH_BB:bb[0-9]+]]
//
// CHECK: [[CRASH_BB]]:
// CHECK-NOT: hop_to_executor {{%[0-9]+}}
// CHECK: unreachable
//
// CHECK: [[SOME_BB]]:
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr %11 : $*Optional<Container>, #Optional.some!enumelt
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr %22 : $*Container, #Container.iso
// CHECK: [[DATA_ADDR:%[0-9]+]] = unchecked_take_enum_data_addr [[ACCESS]] : $*Optional<Container>, #Optional.some!enumelt
// CHECK: [[ELEM_ADDR:%[0-9]+]] = struct_element_addr [[DATA_ADDR]] : $*Container, #Container.iso
// CHECK: [[PREV_AGAIN:%[0-9]+]] = builtin "getCurrentExecutor"() : $Optional<Builtin.Executor>
// CHECK: hop_to_executor {{%[0-9]+}} : $Cat
// CHECK: {{%[0-9]+}} = load [copy] [[ELEM_ADDR]] : $*CatBox
Expand Down Expand Up @@ -644,3 +667,27 @@ struct Blah {
}
}
}

@MainActor
func getTemperature() -> Int { return 0 }

@MainActor
class Polar {
static var temperature: Int = getTemperature()
}


// CHECK-LABEL: sil hidden{{.*}} @$s4test20accessStaticIsolatedSiyYaF : $@convention(thin) @async () -> Int {
// CHECK: [[ADDRESSOR:%[0-9]+]] = function_ref @$s4test5PolarC11temperatureSivau : $@convention(thin) () -> Builtin.RawPointer
// CHECK: hop_to_executor {{%.*}} : $MainActor
// CHECK-NEXT: [[RAW_ADDR:%[0-9]+]] = apply [[ADDRESSOR]]() : $@convention(thin) () -> Builtin.RawPointer
// CHECK-NEXT: hop_to_executor {{%.*}} : $Optional<Builtin.Executor>
// CHECK: [[ADDR:%[0-9]+]] = pointer_to_address [[RAW_ADDR]] : $Builtin.RawPointer to [strict] $*Int
// CHECK: hop_to_executor {{%.*}} : $MainActor
// CHECK: {{%.*}} = load [trivial] {{%.*}} : $*Int
// CHECK: hop_to_executor {{%.*}} : $Optional<Builtin.Executor>
func accessStaticIsolated() async -> Int {
return await Polar.temperature
}