Merge pull request #82961 from gottesmm/pr-b3685ba26b885701787f7b6087dc7d9d873aa500

[rbi] Teach SendNonSendable how to more aggressively suppress sending errors around obfuscated Sendable functions

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -8701,14 +8701,6 @@ ERROR(inherit_actor_context_only_on_async_or_isolation_erased_params,none,
       "asynchronous function types",
       (DeclAttribute))
 
-ERROR(inherit_actor_context_with_concurrent,none,
-      "'@_inheritActorContext' attribute cannot be used together with %0",
-      (const TypeAttribute *))
-
-ERROR(inherit_actor_context_with_nonisolated_nonsending,none,
-      "'@_inheritActorContext' attribute cannot be used together with %0",
-      (TypeRepr *))
-
 //===----------------------------------------------------------------------===//
 // MARK: @concurrent and nonisolated(nonsending) attributes
 //===----------------------------------------------------------------------===//

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -1601,9 +1601,32 @@ struct PartitionOpEvaluator {
   }
 
 private:
-  bool isConvertFunctionFromSendableType(SILValue equivalenceClassRep) const {
+  /// To work around not having isolation in interface types, the type checker
+  /// inserts casts and other AST nodes that are used to enrich the AST with
+  /// isolation information. This results in Sendable functions being
+  /// wrapped/converted/etc in ways that hide the Sendability. This helper looks
+  /// through these conversions/wrappers/thunks to see if the original
+  /// underlying function is Sendable.
+  ///
+  /// The two ways this can happen is that we either get an actual function_ref
+  /// that is Sendable or we get a convert function with a Sendable operand.
+  bool isHiddenSendableFunctionType(SILValue equivalenceClassRep) const {
     SILValue valueToTest = equivalenceClassRep;
     while (true) {
+      if (auto *pai = dyn_cast<PartialApplyInst>(valueToTest)) {
+        if (auto *calleeFunction = pai->getCalleeFunction()) {
+          if (pai->getNumArguments() >= 1 &&
+              pai->getArgument(0)->getType().isFunction() &&
+              calleeFunction->isThunk()) {
+            valueToTest = pai->getArgument(0);
+            continue;
+          }
+
+          if (calleeFunction->getLoweredFunctionType()->isSendable())
+            return true;
+        }
+      }
+
       if (auto *i = dyn_cast<ThinToThickFunctionInst>(valueToTest)) {
         valueToTest = i->getOperand();
         continue;
@@ -1615,6 +1638,9 @@ struct PartitionOpEvaluator {
       break;
     }
 
+    if (auto *fn = dyn_cast<FunctionRefInst>(valueToTest))
+      return fn->getReferencedFunction()->getLoweredFunctionType()->isSendable();
+
     auto *cvi = dyn_cast<ConvertFunctionInst>(valueToTest);
     if (!cvi)
       return false;
@@ -1647,7 +1673,7 @@ struct PartitionOpEvaluator {
 
         // See if we have a convert function from a `@Sendable` type. In this
         // case, we want to squelch the error.
-        if (isConvertFunctionFromSendableType(equivalenceClassRep))
+        if (isHiddenSendableFunctionType(equivalenceClassRep))
           return;
       }
 
@@ -1692,7 +1718,7 @@ struct PartitionOpEvaluator {
 
         // See if we have a convert function from a `@Sendable` type. In this
         // case, we want to squelch the error.
-        if (isConvertFunctionFromSendableType(equivalenceClassRep))
+        if (isHiddenSendableFunctionType(equivalenceClassRep))
           return;
       }
     }

lib/Sema/TypeCheckAttr.cpp

@@ -7856,9 +7856,6 @@ void AttributeChecker::visitInheritActorContextAttr(
     return;
 
   auto paramTy = P->getInterfaceType();
-  if (paramTy->hasError())
-    return;
-
   auto *funcTy =
       paramTy->lookThroughAllOptionalTypes()->getAs<AnyFunctionType>();
   if (!funcTy) {

lib/Sema/TypeCheckDecl.cpp

@@ -2337,9 +2337,6 @@ static Type validateParameterType(ParamDecl *decl) {
   if (dc->isInSpecializeExtensionContext())
     options |= TypeResolutionFlags::AllowUsableFromInline;
 
-  if (decl->getAttrs().hasAttribute<InheritActorContextAttr>())
-    options |= TypeResolutionFlags::InheritsActorContext;
-
   Type Ty;
 
   auto *nestedRepr = decl->getTypeRepr();

lib/Sema/TypeCheckType.cpp

@@ -4217,11 +4217,6 @@ NeverNullType TypeResolver::resolveASTFunctionType(
                       attr->getAttrName());
     }
 
-    if (options.contains(TypeResolutionFlags::InheritsActorContext)) {
-      diagnoseInvalid(repr, attr->getAttrLoc(),
-                      diag::inherit_actor_context_with_concurrent, attr);
-    }
-
     switch (isolation.getKind()) {
     case FunctionTypeIsolation::Kind::NonIsolated:
       break;
@@ -4268,20 +4263,18 @@ NeverNullType TypeResolver::resolveASTFunctionType(
     if (!repr->isInvalid())
       isolation = FunctionTypeIsolation::forNonIsolated();
   } else if (!getWithoutClaiming<CallerIsolatedTypeRepr>(attrs)) {
-    if (!options.contains(TypeResolutionFlags::InheritsActorContext)) {
-      // Infer async function type as `nonisolated(nonsending)` if there is
-      // no `@concurrent` or `nonisolated(nonsending)` attribute and isolation
-      // is nonisolated.
-      if (ctx.LangOpts.hasFeature(Feature::NonisolatedNonsendingByDefault) &&
-          repr->isAsync() && isolation.isNonIsolated()) {
-        isolation = FunctionTypeIsolation::forNonIsolatedCaller();
-      } else if (ctx.LangOpts
-                     .getFeatureState(Feature::NonisolatedNonsendingByDefault)
-                     .isEnabledForMigration()) {
-        // Diagnose only in the interface stage, which is run once.
-        if (inStage(TypeResolutionStage::Interface)) {
-          warnAboutNewNonisolatedAsyncExecutionBehavior(ctx, repr, isolation);
-        }
+    // Infer async function type as `nonisolated(nonsending)` if there is
+    // no `@concurrent` or `nonisolated(nonsending)` attribute and isolation
+    // is nonisolated.
+    if (ctx.LangOpts.hasFeature(Feature::NonisolatedNonsendingByDefault) &&
+        repr->isAsync() && isolation.isNonIsolated()) {
+      isolation = FunctionTypeIsolation::forNonIsolatedCaller();
+    } else if (ctx.LangOpts
+                   .getFeatureState(Feature::NonisolatedNonsendingByDefault)
+                   .isEnabledForMigration()) {
+      // Diagnose only in the interface stage, which is run once.
+      if (inStage(TypeResolutionStage::Interface)) {
+        warnAboutNewNonisolatedAsyncExecutionBehavior(ctx, repr, isolation);
       }
     }
   }
@@ -5331,12 +5324,6 @@ TypeResolver::resolveCallerIsolatedTypeRepr(CallerIsolatedTypeRepr *repr,
     return ErrorType::get(getASTContext());
   }
 
-  if (options.contains(TypeResolutionFlags::InheritsActorContext)) {
-    diagnoseInvalid(repr, repr->getLoc(),
-                    diag::inherit_actor_context_with_nonisolated_nonsending,
-                    repr);
-  }
-
   if (!fnType->isAsync()) {
     diagnoseInvalid(repr, repr->getStartLoc(),
                     diag::nonisolated_nonsending_only_on_async, repr);

lib/Sema/TypeCheckType.h

@@ -87,9 +87,6 @@ enum class TypeResolutionFlags : uint16_t {
 
   /// Whether the immediate context has an @escaping attribute.
   DirectEscaping = 1 << 14,
-
-  /// We are in a `@_inheritActorContext` parameter declaration.
-  InheritsActorContext = 1 << 15,
 };
 
 /// Type resolution contexts that require special handling.

test/Concurrency/attr_execution/attr_execution.swift

@@ -80,16 +80,3 @@ func testClosure() {
   takesClosure {
   }
 }
-
-// CHECK-LABEL: // testInheritsActor(fn:)
-// CHECK: Isolation: global_actor. type: MainActor
-// CHECK: sil hidden [ossa] @$s14attr_execution17testInheritsActor2fnyyyYaYbXE_tYaF : $@convention(thin) @async (@guaranteed @noescape @Sendable @async @callee_guaranteed () -> ()) -> ()
-// CHECK: bb0([[FN:%.*]] : @guaranteed $@noescape @Sendable @async @callee_guaranteed () -> ()):
-// CHECK:  [[FN_COPY:%.*]] = copy_value [[FN]]
-// CHECK:  [[BORROWED_FN_COPY:%.*]] = begin_borrow [[FN_COPY]]
-// CHECK:  apply [[BORROWED_FN_COPY]]() : $@noescape @Sendable @async @callee_guaranteed () -> ()
-// CHECK: } // end sil function '$s14attr_execution17testInheritsActor2fnyyyYaYbXE_tYaF'
-@MainActor
-func testInheritsActor(@_inheritActorContext(always) fn: @Sendable () async -> Void) async {
-  await fn()
-}

test/Concurrency/attr_execution/migration_mode.swift

@@ -393,9 +393,3 @@ do {
     }
   }
 }
-
-// @_inheritActorContext prevents `nonisolated(nonsending)` inference.
-do {
-  func testInherit1(@_inheritActorContext _: @Sendable () async -> Void) {}
-  func testInherit2(@_inheritActorContext(always) _: (@Sendable () async -> Void)?) {}
-}

test/Concurrency/attr_execution/nonisolated_nonsending_by_default.swift

@@ -8,8 +8,3 @@ func testCasts() {
   // expected-error@-1 {{cannot convert value of type '(nonisolated(nonsending) () async -> ()).Type' to type '(() async -> ()).Type' in coercion}}
   _ = defaultedType as (nonisolated(nonsending) () async -> ()).Type // Ok
 }
-
-func test(@_inheritActorContext fn: @Sendable () async -> Void) {
-  let _: Int = fn
-  // expected-error@-1 {{cannot convert value of type '@Sendable () async -> Void' to specified type 'Int'}}
-}

test/Concurrency/transfernonsendable_global_actor_sending.swift

@@ -34,9 +34,8 @@ func useValue<T>(_ t: T) {}
 @MainActor func testGlobalFakeInit() {
   let ns = NonSendableKlass()
 
-  // Will be resolved once @MainActor is @Sendable.
-  Task.fakeInit { @MainActor in // expected-error {{passing closure as a 'sending' parameter risks causing data races between main actor-isolated code and concurrent execution of the closure}}
-    print(ns) // expected-note {{closure captures 'ns' which is accessible to main actor-isolated code}}
+  Task.fakeInit { @MainActor in
+    print(ns)
   }
 
   useValue(ns)