Merge pull request #79382 from xedin/more-Sendable-to-Any-problems

[Concurrency] Fix a few issues with `Senable` and `Any`

Author: xedin

lib/Sema/CSApply.cpp

@@ -7414,6 +7414,19 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
                                             /*isImplicit*/ true));
   }
 
+  case TypeKind::InOut: {
+    auto *inOutExpr = getAsExpr<InOutExpr>(expr);
+    if (!inOutExpr)
+      break;
+
+    // If there is an `any Sendable` -> `Any` mismatch here,
+    // the conversion should be performed on l-value and the
+    // address taken from that. This is something that is already
+    // done as part of implicit `inout` injection for operators
+    // and could be reused here.
+    return coerceToType(inOutExpr->getSubExpr(), toType, locator);
+  }
+
   case TypeKind::Pack:
   case TypeKind::PackElement: {
     llvm_unreachable("Unimplemented!");
@@ -7777,7 +7790,6 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
 #define TYPE(Name, Parent)
 #include "swift/AST/TypeNodes.def"
   case TypeKind::Error:
-  case TypeKind::InOut:
   case TypeKind::Module:
   case TypeKind::Enum:
   case TypeKind::Struct:

lib/Sema/CSSimplify.cpp

@@ -3733,7 +3733,8 @@ static bool matchSendableExistentialToAnyInGenericArgumentPosition(
 
     for (unsigned i = 0, n = path.size(); i < n; ++i) {
       const auto &elt = path[i];
-      if (elt.is<LocatorPathElt::GenericType>()) {
+      if (elt.is<LocatorPathElt::GenericType>() ||
+          elt.is<LocatorPathElt::LValueConversion>()) {
         if (!dropFromIdx)
           dropFromIdx = i;
         continue;
@@ -3774,6 +3775,12 @@ static bool matchSendableExistentialToAnyInGenericArgumentPosition(
           return isPreconcurrencyContext(
               cs.getConstraintLocator(simplifyLocatorToAnchor(locator)));
 
+        if (locator->directlyAt<InOutExpr>()) {
+          auto *IOE = castToExpr<InOutExpr>(locator->getAnchor());
+          return isPreconcurrencyContext(
+              cs.getConstraintLocator(IOE->getSubExpr()));
+        }
+
         auto *calleeLoc = cs.getCalleeLocator(locator);
         if (!calleeLoc)
           return false;

lib/Sema/MiscDiagnostics.cpp

@@ -4351,6 +4351,9 @@ void VarDeclUsageChecker::markStoredOrInOutExpr(Expr *E, unsigned Flags) {
     if (auto *expr = OpaqueValueMap[OVE])
       return markStoredOrInOutExpr(expr, Flags);
 
+  if (auto *ABIConv = dyn_cast<ABISafeConversionExpr>(E))
+    return markStoredOrInOutExpr(ABIConv->getSubExpr(), Flags);
+
   // If we don't know what kind of expression this is, assume it's a reference
   // and mark it as a read.
   E->walk(*this);

lib/Sema/TypeCheckAvailability.cpp

@@ -3593,17 +3593,9 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker {
 
     ExprStack.push_back(E);
 
-    if (auto *apply = dyn_cast<ApplyExpr>(E)) {
-      bool preconcurrency = false;
-      auto *fn = apply->getFn();
-      if (auto *selfApply = dyn_cast<SelfApplyExpr>(fn)) {
-        fn = selfApply->getFn();
-      }
-      auto declRef = fn->getReferencedDecl();
-      if (auto *decl = declRef.getDecl()) {
-        preconcurrency = decl->preconcurrency();
-      }
-      PreconcurrencyCalleeStack.push_back(preconcurrency);
+    if (isa<ApplyExpr>(E)) {
+      PreconcurrencyCalleeStack.push_back(
+          hasReferenceToPreconcurrencyDecl(E));
     }
 
     if (auto DR = dyn_cast<DeclRefExpr>(E)) {
@@ -3629,6 +3621,8 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker {
       if (S->hasDecl()) {
         diagnoseDeclRefAvailability(S->getDecl(), S->getSourceRange(), S);
         maybeDiagStorageAccess(S->getDecl().getDecl(), S->getSourceRange(), DC);
+        PreconcurrencyCalleeStack.push_back(
+            hasReferenceToPreconcurrencyDecl(S));
       }
     }
 
@@ -3677,6 +3671,11 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker {
       maybeDiagKeyPath(KP);
     }
     if (auto A = dyn_cast<AssignExpr>(E)) {
+      // Attempting to assign to a @preconcurrency declaration should
+      // downgrade Sendable conformance mismatches to warnings.
+      PreconcurrencyCalleeStack.push_back(
+        hasReferenceToPreconcurrencyDecl(A->getDest()));
+
       walkAssignExpr(A);
       return Action::SkipChildren(E);
     }
@@ -4007,6 +4006,41 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker {
                                  Flags))
       return;
   }
+
+  /// Check whether the given expression references any
+  /// @preconcurrency declarations.
+  /// Calls, subscripts, member references can have @preconcurrency
+  /// declarations at any point in their base chain.
+  bool hasReferenceToPreconcurrencyDecl(Expr *expr) {
+    if (auto declRef = expr->getReferencedDecl()) {
+      if (declRef.getDecl()->preconcurrency())
+        return true;
+    }
+
+    if (auto *selfApply = dyn_cast<SelfApplyExpr>(expr)) {
+      if (hasReferenceToPreconcurrencyDecl(selfApply->getFn()))
+        return true;
+
+      // Base could be a preconcurrency declaration i.e.
+      //
+      // @preconcurrency var x: [any Sendable]
+      // x.append(...)
+      //
+      // If thought `append` might not be `@preconcurrency`
+      // the "base" is.
+      return hasReferenceToPreconcurrencyDecl(selfApply->getBase());
+    }
+
+    if (auto *LE = dyn_cast<LookupExpr>(expr)) {
+      // If subscript itself is not @preconcurrency, it's base could be.
+      return hasReferenceToPreconcurrencyDecl(LE->getBase());
+    }
+
+    if (auto *apply = dyn_cast<ApplyExpr>(expr))
+      return hasReferenceToPreconcurrencyDecl(apply->getFn());
+
+    return false;
+  }
 };
 } // end anonymous namespace
 

test/Concurrency/predates_concurrency_swift6.swift

@@ -289,3 +289,25 @@ func testErasureDowngrade(ns: NotSendable, us: UnavailableSendable, c: C) {
     // expected-error@-1 {{conformance of 'UnavailableSendable' to 'Sendable' is unavailable}}
   }
 }
+
+// The member itself could be non-preconcurrency but the base could be.
+do {
+  @preconcurrency var d: [String: any Sendable] = [:]
+
+  let data: [String: Any] = [:]
+  d.merge(data, uniquingKeysWith: { _, rhs in rhs})
+  // expected-warning@-1 {{type 'Any' does not conform to the 'Sendable' protocol}}
+
+  struct Test {
+    @preconcurrency var info: [String: any Sendable] = [:]
+  }
+
+  func test(s: inout Test) {
+    s.info["hello"] = { }
+    // expected-warning@-1 {{type '() -> ()' does not conform to the 'Sendable' protocol}}
+    // expected-note@-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+  }
+
+  // If destination is @preconcurrency the Sendable conformance error should be downgraded
+  d = data // expected-warning {{type 'Any' does not conform to the 'Sendable' protocol}}
+}

test/Concurrency/sendable_to_any_for_generic_arguments.swift

@@ -189,3 +189,15 @@ extension User {
     // expected-error@-1 {{referencing initializer 'init(age:)' on '[String : any Sendable].Type' requires the types 'any Sendable' and 'Any' be equivalent}}
   }
 }
+
+// https://github.com/swiftlang/swift/issues/79361
+do {
+  @preconcurrency var d = Dictionary<String, any Sendable>()
+  
+  func test(_ dict: inout Dictionary<String, Any>) {}
+  test(&d) // Ok
+
+  @preconcurrency var a = Array<any Sendable>()
+  let values: [Any] = []
+  a += values // Ok
+}

test/Interpreter/sendable_erasure_to_any_in_preconcurrency.swift

@@ -41,6 +41,13 @@ struct Test {
   @preconcurrency var funcRef: S<([any Sendable]) -> (any Sendable)?> = S(v: { $0.first })
 }
 
+func testInOut(_ dict: inout Dictionary<String, Any>) {
+  dict["inout"] = "yes"
+}
+
+func testInOut(_ arr: inout [Any]) {
+  arr.append("inout")
+}
 
 func test() {
   var c = C()
@@ -98,6 +105,19 @@ func test() {
 
   expectsFuncAny(v1.funcRef)
   // CHECK: 42
+
+  testInOut(&c.dict)
+  print(c.dict["inout"] ?? "no")
+  // CHECK: yes
+
+  testInOut(&c.arr)
+  print(c.arr.contains(where: { $0 as? String == "inout" }))
+  // CHECK: true
+
+  var newValues: [Any] = ["other inout"]
+  c.arr += newValues // checks implicit inout injection via operator
+  print(c.arr.contains(where: { $0 as? String == "other inout" }))
+  // CHECK: true
 }
 
 test()

test/SILGen/sendable_to_any_for_generic_arguments.swift

@@ -261,3 +261,24 @@ func test_subscript_computed_property_and_mutating_access(u: User) {
   // CHECK-NEXT: assign [[COPIED_SENDABLE_DICT]] to [[DICT]]
   u.dict.testMutating()
 }
+
+// CHECK-LABEL: sil hidden [ossa] @$s37sendable_to_any_for_generic_arguments15test_inout_usesyyF
+// CHECK: [[SENDABLE_ARR_REF:%.*]] = begin_access [modify] [unknown] %2
+// CHECK-NEXT:  [[ANY_ARR:%.*]] = alloc_stack $Array<Any>
+// CHECK-NEXT:  [[SENDABLE_ARR:%.*]] = load [copy] [[SENDABLE_ARR_REF]]
+// CHECK-NEXT:  [[ANY_ARR_CAST:%.*]] = unchecked_bitwise_cast [[SENDABLE_ARR]] to $Array<Any>
+// CHECK-NEXT:  [[ANY_ARR_COPY:%.*]] = copy_value [[ANY_ARR_CAST]]
+// CHECK-NEXT:  store [[ANY_ARR_COPY]] to [init] [[ANY_ARR]]
+// CHECK: [[INOUT_FUNC:%.*]] = function_ref @$s37sendable_to_any_for_generic_arguments15test_inout_usesyyF0G0L_yySayypGzF : $@convention(thin) (@inout Array<Any>) -> ()
+// CHECK-NEXT:  {{.*}} = apply [[INOUT_FUNC]]([[ANY_ARR]]) : $@convention(thin) (@inout Array<Any>) -> ()
+// CHECK-NEXT: [[ANY_ARR_VALUE:%.*]] = load [take] [[ANY_ARR]]
+// CHECK-NEXT: [[SENDABLE_ARR_VALUE:%.*]] = unchecked_bitwise_cast [[ANY_ARR_VALUE]] to $Array<any Sendable>
+// CHECK-NEXT: [[SENDABLE_ARR_VALUE_COPY:%.*]] = copy_value [[SENDABLE_ARR_VALUE]]
+// CHECK-NEXT: assign [[SENDABLE_ARR_VALUE_COPY]] to [[SENDABLE_ARR_REF]]
+func test_inout_uses() {
+  func test(_ arr: inout [Any]) {
+  }
+
+  @preconcurrency var arr: [any Sendable] = []
+  test(&arr)
+}