Skip to content

Commit 1343f6b

Browse files
committed
fix busted assumption about decl context
When emitting the note to point out what introduced nonisolation, the code building the message assumed that `DeclContext::getDecl` will not return null, when it can if it's a closure expression. fixes rdar://88776902
1 parent f5bc40b commit 1343f6b

File tree

2 files changed

+51
-30
lines changed

2 files changed

+51
-30
lines changed

lib/SILOptimizer/Mandatory/FlowIsolation.cpp

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#define DEBUG_TYPE "flow-isolation"
1414

15+
#include "swift/AST/Expr.h"
1516
#include "swift/AST/ActorIsolation.h"
1617
#include "swift/AST/DiagnosticsSIL.h"
1718
#include "swift/SIL/ApplySite.h"
@@ -335,30 +336,6 @@ SILInstruction *AnalysisInfo::findNonisolatedBlame(SILInstruction* startInst) {
335336
llvm_unreachable("failed to find nonisolated blame.");
336337
}
337338

338-
static ValueDecl *findCallee(ApplySite &apply) {
339-
SILValue callee = apply.getCalleeOrigin();
340-
341-
auto check = [](ValueDecl *decl) -> ValueDecl* {
342-
// if this is an accessor, then return the storage instead.
343-
if (auto accessor = dyn_cast<AccessorDecl>(decl))
344-
return accessor->getStorage();
345-
346-
return decl;
347-
};
348-
349-
if (auto *methInst = dyn_cast<MethodInst>(callee))
350-
return check(methInst->getMember().getDecl());
351-
352-
if (auto *funcInst = dyn_cast<FunctionRefBaseInst>(callee)) {
353-
auto *refFunc = funcInst->getInitiallyReferencedFunction();
354-
if (auto *declCxt = refFunc->getDeclContext())
355-
if (auto *absFn = dyn_cast<AbstractFunctionDecl>(declCxt->getAsDecl()))
356-
return check(absFn);
357-
}
358-
359-
return nullptr;
360-
}
361-
362339
static StringRef verbForInvoking(ValueDecl *value) {
363340
// Only computed properties need a different verb.
364341
if (isa<AbstractStorageDecl>(value))
@@ -377,24 +354,55 @@ describe(SILInstruction *blame) {
377354

378355
// check if it's a call-like thing.
379356
if (auto apply = ApplySite::isa(blame)) {
380-
// NOTE: we can't use ApplySite::getCalleeFunction because it is overly
381-
// precise in finding the specific corresponding SILFunction. We only care
382-
// about describing the referenced AST decl, since that's all programmers
383-
// know.
384-
ValueDecl *callee = findCallee(apply);
357+
/// First, look for a callee declaration.
358+
///
359+
/// We can't use ApplySite::getCalleeFunction because it is overly
360+
/// precise in finding the specific corresponding SILFunction. We only care
361+
/// about describing the referenced AST decl, since that's all programmers
362+
/// know.
363+
364+
ValueDecl *callee = nullptr;
365+
366+
auto inspect = [](ValueDecl *decl) -> ValueDecl* {
367+
// if this is an accessor, then return the storage instead.
368+
if (auto accessor = dyn_cast<AccessorDecl>(decl))
369+
return accessor->getStorage();
370+
371+
return decl;
372+
};
373+
374+
SILValue silCallee = apply.getCalleeOrigin();
375+
if (auto *methInst = dyn_cast<MethodInst>(silCallee))
376+
callee = inspect(methInst->getMember().getDecl());
377+
378+
if (auto *funcInst = dyn_cast<FunctionRefBaseInst>(silCallee)) {
379+
auto *refFunc = funcInst->getInitiallyReferencedFunction();
380+
381+
if (auto *declCxt = refFunc->getDeclContext()) {
382+
if (auto *absFn =
383+
dyn_cast_or_null<AbstractFunctionDecl>(declCxt->getAsDecl())) {
384+
callee = inspect(absFn);
385+
} else if (isa<AbstractClosureExpr>(declCxt)) {
386+
// TODO: determine if the closure captures self, or is applied to it,
387+
// so we can be more specific in this message.
388+
return std::make_tuple("this closure involving", "", ctx.Id_self);
389+
}
390+
}
391+
}
385392

386393
// if we have no callee info, all we know is it's a call involving self.
387394
if (!callee)
388395
return std::make_tuple("a call involving", "", ctx.Id_self);
389396

397+
// otherwise, form the tuple relative to the callee decl.
390398
return std::make_tuple(
391399
verbForInvoking(callee),
392400
callee->getDescriptiveKindName(callee->getDescriptiveKind()),
393401
callee->getName()
394402
);
395403
}
396404

397-
// handle non-call blames
405+
// handle other non-call blames.
398406
switch (blame->getKind()) {
399407
case SILInstructionKind::CopyValueInst:
400408
return std::make_tuple("making a copy of", "", ctx.Id_self);

test/Concurrency/flow_isolation.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,3 +623,16 @@ actor Rain {
623623
defer { _ = hollerBack(self) }
624624
}
625625
}
626+
627+
@available(SwiftStdlib 5.5, *)
628+
actor OhBrother {
629+
private var giver: () -> Int?
630+
631+
static var DefaultResult: Int { 10 }
632+
633+
init() {
634+
// expected-note@+2 {{after this closure involving 'self', only non-isolated properties of 'self' can be accessed from this init}}
635+
// expected-warning@+1 {{cannot access property 'giver' here in non-isolated initializer; this is an error in Swift 6}}
636+
self.giver = { Self.DefaultResult }
637+
}
638+
}

0 commit comments

Comments
 (0)