Skip to content

[5.5] Restrict uses of 'self' in actor initializers #38043

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
Jun 25, 2021
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
16 changes: 16 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ ERROR(variable_defer_use_uninit,none,
ERROR(self_closure_use_uninit,none,
"'self' captured by a closure before all members were initialized", ())

/// false == sync; true == global-actor isolated
ERROR(self_use_actor_init,none,
"this use of actor 'self' %select{can only|cannot}0 appear in "
"%select{an async|a global-actor isolated}0 initializer",
(bool))
ERROR(self_disallowed_actor_init,none,
"actor 'self' %select{can only|cannot}0 %1 from "
"%select{an async|a global-actor isolated}0 initializer",
(bool, StringRef))
NOTE(actor_convenience_init,none,
"convenience initializers allow non-isolated use of 'self' once "
"initialized",
())




ERROR(variable_addrtaken_before_initialized,none,
"address of %select{variable|constant}1 '%0' taken before it is"
Expand Down
17 changes: 17 additions & 0 deletions lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,23 @@ bool DIMemoryObjectInfo::isElementLetProperty(unsigned Element) const {
return false;
}

ConstructorDecl *DIMemoryObjectInfo::isActorInitSelf() const {
// is it 'self'?
if (!MemoryInst->isVar())
if (auto decl =
dyn_cast_or_null<ClassDecl>(getASTType()->getAnyNominal()))
// is it for an actor?
if (decl->isActor())
if (auto *silFn = MemoryInst->getFunction())
// is it a designated initializer?
if (auto *ctor = dyn_cast_or_null<ConstructorDecl>(
silFn->getDeclContext()->getAsDecl()))
if (ctor->isDesignatedInit())
return ctor;

return nullptr;
}

//===----------------------------------------------------------------------===//
// DIMemoryUse Implementation
//===----------------------------------------------------------------------===//
Expand Down
4 changes: 4 additions & 0 deletions lib/SILOptimizer/Mandatory/DIMemoryUseCollector.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ class DIMemoryObjectInfo {
return false;
}

/// Returns the initializer if the memory use is 'self' and appears in an
/// actor's designated initializer. Otherwise, returns nullptr.
ConstructorDecl *isActorInitSelf() const;

/// True if this memory object is the 'self' of a derived class initializer.
bool isDerivedClassSelf() const { return MemoryInst->isDerivedClassSelf(); }

Expand Down
134 changes: 134 additions & 0 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,13 @@ namespace {

using BlockStates = BasicBlockData<LiveOutBlockState>;

enum class ActorInitKind {
None, // not an actor init
Plain, // synchronous, not isolated to global-actor
PlainAsync, // asynchronous, not isolated to global-actor
GlobalActorIsolated // isolated to global-actor (sync or async).
};

/// LifetimeChecker - This is the main heavy lifting for definite
/// initialization checking of a memory object.
class LifetimeChecker {
Expand Down Expand Up @@ -480,6 +487,21 @@ namespace {
bool diagnoseReturnWithoutInitializingStoredProperties(
const SILInstruction *Inst, SILLocation loc, const DIMemoryUse &Use);

/// Returns true iff the use involves 'self' in a restricted kind of
/// actor initializer. If a non-null Kind pointer was passed in,
/// then the specific kind of restricted actor initializer will be
/// written out. Otherwise, the None initializer kind will be written out.
bool isRestrictedActorInitSelf(const DIMemoryUse& Use,
ActorInitKind *Kind = nullptr) const;

void reportIllegalUseForActorInit(const DIMemoryUse &Use,
ActorInitKind ActorKind,
StringRef ProblemDesc,
bool suggestConvenienceInit) const;

void handleLoadUseFailureForActorInit(const DIMemoryUse &Use,
ActorInitKind ActorKind) const;

void handleLoadUseFailure(const DIMemoryUse &Use,
bool SuperInitDone,
bool FailedSelfUse);
Expand Down Expand Up @@ -866,9 +888,13 @@ void LifetimeChecker::doIt() {

void LifetimeChecker::handleLoadUse(const DIMemoryUse &Use) {
bool IsSuperInitComplete, FailedSelfUse;
ActorInitKind ActorKind = ActorInitKind::None;
// If the value is not definitively initialized, emit an error.
if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
return handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
// Check if it involves 'self' in a restricted actor init.
if (isRestrictedActorInitSelf(Use, &ActorKind))
return handleLoadUseFailureForActorInit(Use, ActorKind);
}

static void replaceValueMetatypeInstWithMetatypeArgument(
Expand Down Expand Up @@ -1177,6 +1203,7 @@ static FullApplySite findApply(SILInstruction *I, bool &isSelfParameter) {

void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
bool IsSuperInitDone, FailedSelfUse;
ActorInitKind ActorKind = ActorInitKind::None;

// inout uses are generally straight-forward: the memory must be initialized
// before the "address" is passed as an l-value.
Expand All @@ -1195,6 +1222,11 @@ void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
return;
}

// 'self' cannot be passed 'inout' from some kinds of actor initializers.
if (isRestrictedActorInitSelf(Use, &ActorKind))
reportIllegalUseForActorInit(Use, ActorKind, "be passed 'inout'",
/*suggestConvenienceInit=*/false);

// One additional check: 'let' properties may never be passed inout, because
// they are only allowed to have their initial value set, not a subsequent
// overwrite.
Expand Down Expand Up @@ -1357,12 +1389,20 @@ static bool isLoadForReturn(SingleValueInstruction *loadInst) {
}

void LifetimeChecker::handleEscapeUse(const DIMemoryUse &Use) {

// The value must be fully initialized at all escape points. If not, diagnose
// the error.
bool SuperInitDone, FailedSelfUse, FullyUninitialized;
ActorInitKind ActorKind = ActorInitKind::None;

if (isInitializedAtUse(Use, &SuperInitDone, &FailedSelfUse,
&FullyUninitialized)) {

// no escaping uses of 'self' are allowed in restricted actor inits.
if (isRestrictedActorInitSelf(Use, &ActorKind))
reportIllegalUseForActorInit(Use, ActorKind, "be captured by a closure",
/*suggestConvenienceInit=*/true);

return;
}

Expand Down Expand Up @@ -1746,6 +1786,100 @@ bool LifetimeChecker::diagnoseReturnWithoutInitializingStoredProperties(
return true;
}

bool LifetimeChecker::isRestrictedActorInitSelf(const DIMemoryUse& Use,
ActorInitKind *Kind) const {

auto result = [&](ActorInitKind k, bool isRestricted) -> bool {
if (Kind)
*Kind = k;
return isRestricted;
};

// Currently: being synchronous, or global-actor isolated, means the actor's
// self is restricted within the init.
if (auto *ctor = TheMemory.isActorInitSelf()) {
if (getActorIsolation(ctor).isGlobalActor()) // global-actor isolated?
return result(ActorInitKind::GlobalActorIsolated, true);
else if (!ctor->hasAsync()) // synchronous?
return result(ActorInitKind::Plain, true);
else
return result(ActorInitKind::PlainAsync, false);
}

return result(ActorInitKind::None, false);
}

void LifetimeChecker::reportIllegalUseForActorInit(
const DIMemoryUse &Use,
ActorInitKind ActorKind,
StringRef ProblemDesc,
bool suggestConvenienceInit) const {
switch(ActorKind) {
case ActorInitKind::None:
case ActorInitKind::PlainAsync:
llvm::report_fatal_error("this actor init is never problematic!");

case ActorInitKind::Plain:
diagnose(Module, Use.Inst->getLoc(), diag::self_disallowed_actor_init,
false, ProblemDesc);
break;

case ActorInitKind::GlobalActorIsolated:
diagnose(Module, Use.Inst->getLoc(), diag::self_disallowed_actor_init,
true, ProblemDesc);
break;
}

if (suggestConvenienceInit)
diagnose(Module, Use.Inst->getLoc(), diag::actor_convenience_init);
}

void LifetimeChecker::handleLoadUseFailureForActorInit(
const DIMemoryUse &Use,
ActorInitKind ActorKind) const {
assert(TheMemory.isAnyInitSelf());
SILInstruction *Inst = Use.Inst;

// While enum instructions have no side-effects, they do wrap-up `self`
// such that it can escape our analysis. So, enum instruction uses are only
// acceptable if the enum is part of a specific pattern of usage that is
// generated for a failable initializer.
if (auto *enumInst = dyn_cast<EnumInst>(Inst)) {
if (isFailableInitReturnUseOfEnum(enumInst))
return;

// Otherwise, if the use has no possibility of side-effects, then its OK.
} else if (!Inst->mayHaveSideEffects()) {
return;

// While loads can have side-effects, we are not concerned by them here.
} else if (isa<LoadInst>(Inst)) {
return;
}

// Everything else is disallowed!
switch(ActorKind) {
case ActorInitKind::None:
case ActorInitKind::PlainAsync:
llvm::report_fatal_error("this actor init is never problematic!");

case ActorInitKind::Plain:
diagnose(Module, Use.Inst->getLoc(), diag::self_use_actor_init, false);
break;

case ActorInitKind::GlobalActorIsolated:
diagnose(Module, Use.Inst->getLoc(), diag::self_use_actor_init, true);
break;
}

// We cannot easily determine which argument in the call the use of 'self'
// appears in. If we could, then we could determine whether the callee
// is 'isolated' to that parameter, in order to avoid suggesting a convenience
// init in those cases. Thus, the phrasing of the note should be informative.
if (isa<ApplyInst>(Inst))
diagnose(Module, Use.Inst->getLoc(), diag::actor_convenience_init);
}

/// Check and diagnose various failures when a load use is not fully
/// initialized.
///
Expand Down
Loading