Merge pull request #41375 from kavon/unsegfault-flowiso

fix flow-isolation diagnostic message crash

Author: swift-ci

lib/SILOptimizer/Mandatory/FlowIsolation.cpp

@@ -12,6 +12,7 @@
 
 #define DEBUG_TYPE "flow-isolation"
 
+#include "swift/AST/Expr.h"
 #include "swift/AST/ActorIsolation.h"
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/SIL/ApplySite.h"
@@ -335,30 +336,6 @@ SILInstruction *AnalysisInfo::findNonisolatedBlame(SILInstruction* startInst) {
   llvm_unreachable("failed to find nonisolated blame.");
 }
 
-static ValueDecl *findCallee(ApplySite &apply) {
-  SILValue callee = apply.getCalleeOrigin();
-
-  auto check = [](ValueDecl *decl) -> ValueDecl* {
-    // if this is an accessor, then return the storage instead.
-    if (auto accessor = dyn_cast<AccessorDecl>(decl))
-      return accessor->getStorage();
-
-    return decl;
-  };
-
-  if (auto *methInst = dyn_cast<MethodInst>(callee))
-    return check(methInst->getMember().getDecl());
-
-  if (auto *funcInst = dyn_cast<FunctionRefBaseInst>(callee)) {
-    auto *refFunc = funcInst->getInitiallyReferencedFunction();
-    if (auto *declCxt = refFunc->getDeclContext())
-      if (auto *absFn = dyn_cast<AbstractFunctionDecl>(declCxt->getAsDecl()))
-        return check(absFn);
-  }
-
-  return nullptr;
-}
-
 static StringRef verbForInvoking(ValueDecl *value) {
   // Only computed properties need a different verb.
   if (isa<AbstractStorageDecl>(value))
@@ -377,24 +354,55 @@ describe(SILInstruction *blame) {
 
   // check if it's a call-like thing.
   if (auto apply = ApplySite::isa(blame)) {
-    // NOTE: we can't use ApplySite::getCalleeFunction because it is overly
-    // precise in finding the specific corresponding SILFunction. We only care
-    // about describing the referenced AST decl, since that's all programmers
-    // know.
-    ValueDecl *callee = findCallee(apply);
+    /// First, look for a callee declaration.
+    ///
+    /// We can't use ApplySite::getCalleeFunction because it is overly
+    /// precise in finding the specific corresponding SILFunction. We only care
+    /// about describing the referenced AST decl, since that's all programmers
+    /// know.
+
+    ValueDecl *callee = nullptr;
+
+    auto inspect = [](ValueDecl *decl) -> ValueDecl* {
+      // if this is an accessor, then return the storage instead.
+      if (auto accessor = dyn_cast<AccessorDecl>(decl))
+        return accessor->getStorage();
+
+      return decl;
+    };
+
+    SILValue silCallee = apply.getCalleeOrigin();
+    if (auto *methInst = dyn_cast<MethodInst>(silCallee))
+      callee = inspect(methInst->getMember().getDecl());
+
+    if (auto *funcInst = dyn_cast<FunctionRefBaseInst>(silCallee)) {
+      auto *refFunc = funcInst->getInitiallyReferencedFunction();
+
+      if (auto *declCxt = refFunc->getDeclContext()) {
+        if (auto *absFn =
+            dyn_cast_or_null<AbstractFunctionDecl>(declCxt->getAsDecl())) {
+          callee = inspect(absFn);
+        } else if (isa<AbstractClosureExpr>(declCxt)) {
+          // TODO: determine if the closure captures self, or is applied to it,
+          // so we can be more specific in this message.
+          return std::make_tuple("this closure involving", "", ctx.Id_self);
+        }
+      }
+    }
 
     // if we have no callee info, all we know is it's a call involving self.
     if (!callee)
       return std::make_tuple("a call involving", "", ctx.Id_self);
 
+    // otherwise, form the tuple relative to the callee decl.
     return std::make_tuple(
       verbForInvoking(callee),
       callee->getDescriptiveKindName(callee->getDescriptiveKind()),
       callee->getName()
     );
   }
 
-  // handle non-call blames
+  // handle other non-call blames.
   switch (blame->getKind()) {
     case SILInstructionKind::CopyValueInst:
       return std::make_tuple("making a copy of", "", ctx.Id_self);

test/Concurrency/flow_isolation.swift

@@ -653,3 +653,38 @@ actor DeinitExceptionForSwift5 {
     x = 1 // expected-warning {{cannot access property 'x' here in deinitializer; this is an error in Swift 6}}
   }
 }
+
+@available(SwiftStdlib 5.5, *)
+actor OhBrother {
+  private var giver: (OhBrother) -> Int
+  private var whatever: Int = 0
+
+  static var DefaultResult: Int { 10 }
+
+  init() {
+    // expected-note@+2 {{after this closure involving 'self', only non-isolated properties of 'self' can be accessed from this init}}
+    // expected-warning@+1 {{cannot access property 'giver' here in non-isolated initializer; this is an error in Swift 6}}
+    self.giver = { (x: OhBrother) -> Int in Self.DefaultResult }
+  }
+
+  init(v2: Void) {
+    giver = { (x: OhBrother) -> Int in 0 }
+
+    // make sure we don't call this a closure, which is the more common situation.
+
+    _ = giver(self) // expected-note {{after a call involving 'self', only non-isolated properties of 'self' can be accessed from this init}}
+
+    whatever = 1 // expected-warning {{cannot access property 'whatever' here in non-isolated initializer; this is an error in Swift 6}}
+  }
+
+  init(v3: Void) {
+    let blah = { (x: OhBrother) -> Int in 0 }
+    giver = blah
+
+    // TODO: would be nice if we didn't say "after this closure" since it's not a capture, but a call.
+
+    _ = blah(self) // expected-note {{after this closure involving 'self', only non-isolated properties of 'self' can be accessed from this init}}
+
+    whatever = 2 // expected-warning {{cannot access property 'whatever' here in non-isolated initializer; this is an error in Swift 6}}
+  }
+}