[Executors] Specialized message for failed assert between 2 default actors

fix missing default initializer for ExecutorActiveAndRef

Author: ktoso

include/swift/Runtime/Concurrency.h

@@ -897,8 +897,16 @@ TypeNamePair swift_task_getExecutorRefTypeName(ExecutorRef executor);
 /// Since the `nullptr, 0` executor ref is a valid "generic executor" reference,
 /// when we need to know "was the generic one actually set, or are we just defaulting to it
 struct ExecutorActiveAndRef {
-  bool active;
-  ExecutorRef ref;
+  const bool active;
+  const ExecutorRef ref;
+
+  ExecutorActiveAndRef() :
+      active(false),
+      ref(ExecutorRef::generic()) {}
+
+  ExecutorActiveAndRef(bool active, ExecutorRef ref) :
+      active(active),
+      ref(ref) {}
 };
 
 /// Different than `swift_task_getCurrentExecutor` in that it does not default to the generic executor.

stdlib/public/Concurrency/ExecutorAssertions.swift

@@ -49,8 +49,7 @@ func preconditionTaskOnExecutor(
   let expectationCheck = _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor)
 
   precondition(expectationCheck,
-      "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " +
-      "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+      makeUnexpectedExecutorMessage(executor: executor.asUnownedSerialExecutor().executor, message: message()),
       file: file, line: line) // short-cut so we get the exact same failure reporting semantics
 }
 
@@ -86,9 +85,7 @@ func preconditionTaskOnActorExecutor(
   let expectationCheck = _taskIsCurrentExecutor(actor.unownedExecutor.executor)
 
   precondition(expectationCheck,
-      "Expected same executor as actor '\(actor)' " +
-      "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " +
-      "but was executing on '\(_executorGetCurrentActiveExecutorName())'. \(message())",
+      makeUnexpectedExecutorMessage(actor: actor, message: message()),
       file: file, line: line)
 }
 
@@ -123,8 +120,7 @@ func assertTaskOnExecutor(
 
   guard _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) else {
     assertionFailure(
-        "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " +
-        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+        makeUnexpectedExecutorMessage(executor: executor.asUnownedSerialExecutor().executor, message: message()),
         file: file, line: line)
     return
   }
@@ -158,12 +154,9 @@ func assertTaskOnActorExecutor(
   }
 
   guard _taskIsCurrentExecutor(actor.unownedExecutor.executor) else {
-    let msg =
-        "Expected same executor as actor '\(actor)' " +
-        "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " +
-        "but was executing on '\(_executorGetCurrentActiveExecutorName())'. " +
-        "\(message())";
-    assertionFailure(msg, file: file, line: line) // short-cut so we get the exact same failure reporting semantics
+    assertionFailure(
+        makeUnexpectedExecutorMessage(actor: actor, message: message()),
+        file: file, line: line)
     return
   }
 }
@@ -200,8 +193,7 @@ func assumeOnMainActorExecutor<T>(
 
   guard _taskIsCurrentExecutor(mainExecutor) else {
     fatalError(
-        "Expected '\(_executorGetTypeName(mainExecutor))' executor, " +
-        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+        makeUnexpectedExecutorMessage(executor: mainExecutor, message: nil),
         file: file, line: line)
   }
 
@@ -241,10 +233,7 @@ func assumeOnActorExecutor<Act: Actor, T>(
   /// as this is our "safe" version of this API.
   let executor: Builtin.Executor = actor.unownedExecutor.executor
   guard _taskIsCurrentExecutor(executor) else {
-    fatalError(
-        "Expected same executor as actor '\(actor)' " +
-        "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " +
-        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+    fatalError(makeUnexpectedExecutorMessage(actor: actor, message: nil),
         file: file, line: line)
   }
 
@@ -260,7 +249,7 @@ func assumeOnActorExecutor<Act: Actor, T>(
 /// I.e. if we're not executing within a task, we expect to have `<unknown>` executor rather than the generic one.
 @usableFromInline
 @available(SwiftStdlib 5.9, *)
-func _executorGetCurrentActiveExecutorName() -> String {
+internal func _executorGetCurrentActiveExecutorName() -> String {
   let (wasActive, ref) = _executorGetCurrentActiveExecutorRef()
   guard wasActive else {
     return "<unknown>"
@@ -269,5 +258,57 @@ func _executorGetCurrentActiveExecutorName() -> String {
   return _executorGetTypeName(ref)
 }
 
+@usableFromInline
+@available(SwiftStdlib 5.9, *)
+internal func makeUnexpectedExecutorMessage(actor: some Actor, message: String?) -> String {
+  let expectedExecutor = _executorGetTypeName(actor.unownedExecutor.executor)
+  let gotExecutor = _executorGetCurrentActiveExecutorName()
+
+  let sameExecutorNameClarification: String
+  if expectedExecutor == gotExecutor {
+    sameExecutorNameClarification = " of a different actor"
+  } else {
+    sameExecutorNameClarification = ""
+  }
+
+  let trailingMessage: String
+  if let message {
+    trailingMessage = " \(message)"
+  } else {
+    trailingMessage = ""
+  }
+
+  return "Expected same executor as actor '\(actor)' " +
+      "('\(expectedExecutor)'), " +
+      "but was executing on '\(gotExecutor)'" +
+      "\(sameExecutorNameClarification)." +
+      "\(trailingMessage)"
+}
+
+@usableFromInline
+@available(SwiftStdlib 5.9, *)
+internal func makeUnexpectedExecutorMessage(executor: Builtin.Executor, message: String?) -> String {
+  let expectedExecutor = _executorGetTypeName(executor)
+  let gotExecutor = _executorGetCurrentActiveExecutorName()
+
+  let sameExecutorNameClarification: String
+  if expectedExecutor == gotExecutor {
+    sameExecutorNameClarification = " of a different actor"
+  } else {
+    sameExecutorNameClarification = ""
+  }
+
+  let trailingMessage: String
+  if let message {
+    trailingMessage = " \(message)"
+  } else {
+    trailingMessage = ""
+  }
+
+  return "Expected '\(expectedExecutor)' executor, " +
+      "but was executing on '\(gotExecutor)'" +
+      "\(sameExecutorNameClarification)."
+}
+
 // TODO(ktoso): implement assume for distributed actors as well
 #endif // not SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY

test/Concurrency/Runtime/actor_assume_executor_general.swift

@@ -61,6 +61,15 @@ actor DefaultExecutorSomeone {
   }
 }
 
+actor DefaultExecutorSomeoneElse {
+  func checkSameExecutorAs(other: DefaultExecutorSomeone) {
+    // this is expected to fail
+    assumeOnActorExecutor(other) { isolatedOther in
+      let s: String = isolatedOther.something // ok, if the runtime check were to pass
+    }
+  }
+}
+
 actor DefaultExecutorSomeonesFriend {
   let someone: DefaultExecutorSomeone
   nonisolated var unownedExecutor: UnownedSerialExecutor {
@@ -175,6 +184,15 @@ final class MainActorEcho {
         expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on 'a.InlineExecutor'.")
         await InlineExecutorActor(someone: someone).checkMainActor()
       }
+
+      tests.test("assumeOnActorExecutor: wrongly assume 'same executor' as a different default actor, from a different default actor") {
+        // TODO: this message is not very helpful, and we should try to print the exact default actor we executed on,
+        //       however, this is tricky due to the fact a default actor can be destroyed while executing on it, so we 
+        //       must take extra care for how we'd retain and print such default actor. rdar://106487242
+        expectCrashLater(withMessage: "Expected same executor as actor 'a.DefaultExecutorSomeone' ('DefaultActorExecutor'), but was executing on 'DefaultActorExecutor' of a different actor.")
+        let someone = DefaultExecutorSomeone()
+        await DefaultExecutorSomeoneElse().checkSameExecutorAs(other: someone)
+      }
     }
 
     await runAllTestsAsync()