[Executors] improved executor precondition messages

Author: ktoso

include/swift/Runtime/Concurrency.h

@@ -891,6 +891,19 @@ ExecutorRef swift_task_getMainExecutor(void);
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 bool swift_task_isCurrentExecutor(ExecutorRef executor);
 
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+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;
+};
+
+SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
+ExecutorActiveAndRef swift_task_getCurrentActiveExecutorRef();
+
 SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
 void swift_task_reportUnexpectedExecutor(
     const unsigned char *file, uintptr_t fileLength, bool fileIsASCII,

stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def

@@ -124,6 +124,14 @@ OVERRIDE_ACTOR(task_isCurrentExecutor, bool,
                SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
                swift::, (ExecutorRef executor), (executor))
 
+OVERRIDE_ACTOR(task_getExecutorRefTypeName, TypeNamePair,
+               SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
+               swift::, (ExecutorRef executor), (executor))
+
+OVERRIDE_ACTOR(task_getCurrentActiveExecutorRef, ExecutorActiveAndRef,
+               SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
+               swift::, ,)
+
 OVERRIDE_ACTOR(task_switch, void,
                SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swiftasync),
                swift::, (SWIFT_ASYNC_CONTEXT AsyncContext *resumeToContext,

stdlib/public/Concurrency/Actor.cpp

@@ -295,15 +295,47 @@ JobPriority swift::swift_task_getCurrentThreadPriority() {
 #endif
 }
 
+SWIFT_CC(swift)
+static TypeNamePair swift_task_getExecutorRefTypeNameImpl(ExecutorRef ref) {
+  TypeNamePair executorName;
+  if (ref.isDefaultActor()) {
+    auto defaultActorExecutorName = "DefaultActorExecutor";
+    executorName = TypeNamePair{defaultActorExecutorName, strlen(defaultActorExecutorName)};
+  } else if (ref.isMainExecutor()) {
+    auto mainActorExecutorName = "MainActorExecutor";
+    executorName = TypeNamePair{mainActorExecutorName, strlen(mainActorExecutorName)};
+  } else if (ref.isGeneric()) {
+    auto genericExecutorName = "GenericExecutor"; // TODO: what's a better name for it?
+    executorName = TypeNamePair{genericExecutorName, strlen(genericExecutorName)};
+  } else {
+    HeapObject * identity = ref.getIdentity();
+    assert(identity);
+    auto *metadata = identity->metadata;
+    executorName = swift_getTypeName(metadata, /*qualified=*/true);
+  }
+
+  return executorName;
+}
+
 SWIFT_CC(swift)
 static bool swift_task_isCurrentExecutorImpl(ExecutorRef executor) {
   if (auto currentTracking = ExecutorTrackingInfo::current()) {
     return currentTracking->getActiveExecutor() == executor;
   }
 
+  // TODO(ktoso): wrong assumption; we can be on the main queue without being on main thread
   return executor.isMainExecutor() && isExecutingOnMainThread();
 }
 
+SWIFT_CC(swift)
+static ExecutorActiveAndRef swift_task_getCurrentActiveExecutorRefImpl() {
+  if (auto currentTracking = ExecutorTrackingInfo::current()) {
+    return ExecutorActiveAndRef{true, currentTracking->getActiveExecutor()}; // FIXME; special case main and default
+  }
+
+  return ExecutorActiveAndRef{false, ExecutorRef::generic()};
+}
+
 /// Logging level for unexpected executors:
 /// 0 - no logging
 /// 1 - warn on each instance

stdlib/public/Concurrency/ExecutorAssertions.swift

@@ -33,7 +33,8 @@ import SwiftShims
 ///   programming error.
 ///
 /// - Parameter executor: the expected current executor
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
+@_transparent
 public
 func preconditionTaskOnExecutor(
     _ executor: some SerialExecutor,
@@ -46,10 +47,9 @@ func preconditionTaskOnExecutor(
 
   let expectationCheck = _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor)
 
-  /// TODO: implement the logic in-place perhaps rather than delegating to precondition()?
   precondition(expectationCheck,
-      // TODO: offer information which executor we actually got
-      "Incorrect actor executor assumption; Expected '\(executor)' executor. \(message())",
+      "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " +
+      "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
       file: file, line: line) // short-cut so we get the exact same failure reporting semantics
 }
 
@@ -70,7 +70,8 @@ func preconditionTaskOnExecutor(
 ///   programming error.
 ///
 /// - Parameter actor: the actor whose serial executor we expect to be the current executor
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
+@_transparent
 public
 func preconditionTaskOnActorExecutor(
     _ actor: some Actor,
@@ -83,10 +84,10 @@ func preconditionTaskOnActorExecutor(
 
   let expectationCheck = _taskIsCurrentExecutor(actor.unownedExecutor.executor)
 
-  // TODO: offer information which executor we actually got
   precondition(expectationCheck,
-      // TODO: figure out a way to get the typed repr out of the unowned executor
-      "Incorrect actor executor assumption; Expected '\(actor.unownedExecutor)' executor. \(message())",
+      "Expected same executor as actor '\(actor)' " +
+      "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " +
+      "but was executing on '\(_executorGetCurrentActiveExecutorName())'. \(message())",
       file: file, line: line)
 }
 
@@ -107,7 +108,8 @@ func preconditionTaskOnActorExecutor(
 ///   assumption is a serious programming error.
 ///
 /// - Parameter executor: the expected current executor
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
+@_transparent
 public
 func assertTaskOnExecutor(
     _ executor: some SerialExecutor,
@@ -119,10 +121,10 @@ func assertTaskOnExecutor(
   }
 
   guard _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) else {
-    // TODO: offer information which executor we actually got
-    let msg = "Incorrect actor executor assumption; Expected '\(executor)' executor. \(message())"
-    /// TODO: implement the logic in-place perhaps rather than delegating to precondition()?
-    assertionFailure(msg, file: file, line: line)
+    assertionFailure(
+        "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " +
+        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+        file: file, line: line)
     return
   }
 }
@@ -142,7 +144,8 @@ func assertTaskOnExecutor(
 ///
 ///
 /// - Parameter actor: the actor whose serial executor we expect to be the current executor
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
+@_transparent
 public
 func assertTaskOnActorExecutor(
     _ actor: some Actor,
@@ -154,10 +157,11 @@ func assertTaskOnActorExecutor(
   }
 
   guard _taskIsCurrentExecutor(actor.unownedExecutor.executor) else {
-    // TODO: offer information which executor we actually got
-    // TODO: figure out a way to get the typed repr out of the unowned executor
-    let msg = "Incorrect actor executor assumption; Expected '\(actor.unownedExecutor)' executor. \(message())"
-    /// TODO: implement the logic in-place perhaps rather than delegating to precondition()?
+    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
     return
   }
@@ -179,7 +183,7 @@ func assertTaskOnActorExecutor(
 /// if another actor uses the same serial executor--by using ``MainActor/sharedUnownedExecutor``
 /// as its own ``Actor/unownedExecutor``--this check will succeed, as from a concurrency safety
 /// perspective, the serial executor guarantees mutual exclusion of those two actors.
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
 @_unavailableFromAsync(message: "await the call to the @MainActor closure directly")
 public
 func assumeOnMainActorExecutor<T>(
@@ -191,9 +195,13 @@ func assumeOnMainActorExecutor<T>(
 
   /// This is guaranteed to be fatal if the check fails,
   /// as this is our "safe" version of this API.
-  guard _taskIsCurrentExecutor(Builtin.buildMainActorExecutorRef()) else {
-    // TODO: offer information which executor we actually got
-    fatalError("Incorrect actor executor assumption; Expected 'MainActor' executor.", file: file, line: line)
+  let mainExecutor = Builtin.buildMainActorExecutorRef()
+
+  guard _taskIsCurrentExecutor(mainExecutor) else {
+    fatalError(
+        "Expected '\(_executorGetTypeName(mainExecutor))' executor, " +
+        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+        file: file, line: line)
   }
 
   // To do the unsafe cast, we have to pretend it's @escaping.
@@ -217,7 +225,7 @@ func assumeOnMainActorExecutor<T>(
 /// if another actor uses the same serial executor--by using that actor's ``Actor/unownedExecutor``
 /// as its own ``Actor/unownedExecutor``--this check will succeed, as from a concurrency safety
 /// perspective, the serial executor guarantees mutual exclusion of those two actors.
-@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9)
+@available(SwiftStdlib 5.9, *)
 @_unavailableFromAsync(message: "express the closure as an explicit function declared on the specified 'actor' instead")
 public
 func assumeOnActorExecutor<Act: Actor, T>(
@@ -232,8 +240,11 @@ 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 {
-    // TODO: offer information which executor we actually got
-    fatalError("Incorrect actor executor assumption; Expected same executor as \(actor).", file: file, line: line)
+    fatalError(
+        "Expected same executor as actor '\(actor)' " +
+        "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " +
+        "but was executing on '\(_executorGetCurrentActiveExecutorName())'.",
+        file: file, line: line)
   }
 
   // To do the unsafe cast, we have to pretend it's @escaping.
@@ -244,4 +255,17 @@ func assumeOnActorExecutor<Act: Actor, T>(
   }
 }
 
+/// Specifically get the name of the current *active* executor, without falling back to assuming the generic one.
+/// 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 {
+  let (wasActive, ref) = _executorGetCurrentActiveExecutorRef()
+  guard wasActive else {
+    return "<unknown>"
+  }
+
+  return _executorGetTypeName(ref)
+}
+
 // TODO(ktoso): implement assume for distributed actors as well

stdlib/public/Concurrency/Task.swift

@@ -978,6 +978,30 @@ func _taskCreateNullaryContinuationJob(priority: Int, continuation: Builtin.RawU
 @_silgen_name("swift_task_isCurrentExecutor")
 func _taskIsCurrentExecutor(_ executor: Builtin.Executor) -> Bool
 
+@available(SwiftStdlib 5.9, *)
+@usableFromInline
+@_silgen_name("swift_task_getExecutorRefTypeName")
+func _executorGetTypeNameRaw(_ executor: Builtin.Executor) -> (UnsafePointer<UInt8>?, Int)
+
+@available(SwiftStdlib 5.9, *)
+@usableFromInline
+func _executorGetTypeName(_ executor: Builtin.Executor) -> String {
+  let (stringPtr, count) = _executorGetTypeNameRaw(executor)
+  guard let stringPtr else {
+    return "<unknown>"
+  }
+
+  // FIXME: can't use this since it is internal in stdlib and we're in concurrency
+  //  return String._fromUTF8Repairing(
+  //      UnsafeBufferPointer(start: stringPtr, count: count)).0
+  return String(cString: stringPtr)
+}
+
+@available(SwiftStdlib 5.9, *)
+@usableFromInline
+@_silgen_name("swift_task_getCurrentActiveExecutorRef")
+func _executorGetCurrentActiveExecutorRef() -> (Bool, Builtin.Executor)
+
 @available(SwiftStdlib 5.1, *)
 @usableFromInline
 @_silgen_name("swift_task_reportUnexpectedExecutor")

stdlib/public/Concurrency/TaskGroup.swift

@@ -632,6 +632,7 @@ public struct ThrowingTaskGroup<ChildTaskResult: Sendable, Failure: Error> {
   ///     } catch {
   ///         // other errors though we print and cancel the group,
   ///         // and all of the remaining child tasks within it.
+  ///         print("Error: \(error)")
   ///         group.cancelAll()
   ///     }
   /// }

test/Concurrency/Runtime/actor_assert_precondition_executor.swift

@@ -73,7 +73,7 @@ actor Someone {
       }
 
       tests.test("preconditionTaskOnActorExecutor(main): wrongly assume the main executor, from actor on other executor") {
-        expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected 'MainActor' executor.")
+        expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on 'DefaultActorExecutor'.")
         await Someone().callCheckMainActor()
       }