Merge pull request swiftlang#36083 from DougGregor/concurrentvalue-keypaths

Author: swift-ci

include/swift/AST/DiagnosticsSema.def

@@ -4316,6 +4316,9 @@ WARNING(non_concurrent_property_type,none,
         "cannot use %0 %1 with a non-concurrent-value type %2 "
         "%select{across actors|from concurrently-executed code}3",
         (DescriptiveDeclKind, DeclName, Type, bool))
+WARNING(non_concurrent_keypath_capture,none,
+        "cannot form key path that captures non-concurrent-value type %0",
+        (Type))
 ERROR(non_concurrent_type_member,none,
       "%select{stored property %1|associated value %1}0 of "
       "'ConcurrentValue'-conforming %2 %3 has non-concurrent-value type %4",

lib/Sema/TypeCheckConcurrency.cpp

@@ -793,71 +793,6 @@ static bool isConcurrentValueType(const DeclContext *dc, Type type) {
   return checker.visit(type);
 }
 
-Optional<NonConcurrentType> NonConcurrentType::get(
-    const DeclContext *dc, ConcreteDeclRef declRef) {
-  // For functions, check the parameter and result types.
-  SubstitutionMap subs = declRef.getSubstitutions();
-  if (auto function = dyn_cast<AbstractFunctionDecl>(declRef.getDecl())) {
-    for (auto param : *function->getParameters()) {
-      Type paramType = param->getInterfaceType().subst(subs);
-      if (!isConcurrentValueType(dc, paramType)) {
-        return NonConcurrentType {
-            Kind::Parameter, ConcreteDeclRef(param, subs), paramType };
-      }
-    }
-
-    // Check the result type of a function.
-    if (auto func = dyn_cast<FuncDecl>(function)) {
-      Type resultType = func->getResultInterfaceType().subst(subs);
-      if (!isConcurrentValueType(dc, resultType)) {
-        return NonConcurrentType { Kind::Result, declRef, resultType };
-      }
-    }
-
-    // Check the "self" type of an instance method.
-    if (function->isInstanceMember()) {
-      if (auto selfParam = function->getImplicitSelfDecl()) {
-        Type paramType = selfParam->getInterfaceType().subst(subs);
-        if (!isConcurrentValueType(dc, paramType)) {
-          return NonConcurrentType {
-              Kind::Parameter, ConcreteDeclRef(selfParam, subs),
-              paramType };
-        }
-      }
-    }
-  } else if (auto var = dyn_cast<VarDecl>(declRef.getDecl())) {
-    Type propertyType = var->getValueInterfaceType().subst(subs);
-    if (!isConcurrentValueType(dc, propertyType)) {
-      return NonConcurrentType {
-        Kind::Property, declRef, propertyType };
-    }
-  }
-
-  return None;
-}
-
-void NonConcurrentType::diagnose(SourceLoc loc) {
-  ASTContext &ctx = declRef.getDecl()->getASTContext();
-
-  switch (kind) {
-  case Parameter:
-    ctx.Diags.diagnose(loc, diag::non_concurrent_param_type, type);
-    break;
-
-  case Result:
-    ctx.Diags.diagnose(loc, diag::non_concurrent_result_type, type);
-    break;
-
-  case Property: {
-    auto var = cast<VarDecl>(declRef.getDecl());
-    ctx.Diags.diagnose(loc, diag::non_concurrent_property_type,
-                       var->getDescriptiveKind(), var->getName(),
-                       type, var->isLocalCapture());
-    break;
-  }
-  }
-}
-
 static bool diagnoseNonConcurrentParameter(
     SourceLoc loc, ConcurrentReferenceKind refKind, ConcreteDeclRef declRef,
     ParamDecl *param, Type paramType) {
@@ -1176,6 +1111,22 @@ namespace {
         }
       }
 
+      // Key paths require any captured values to be ConcurrentValue-conforming.
+      if (auto keyPath = dyn_cast<KeyPathExpr>(expr)) {
+        for (const auto &component : keyPath->getComponents()) {
+          auto indexExpr = component.getIndexExpr();
+          if (!indexExpr || !indexExpr->getType())
+            continue;
+
+          if (ctx.LangOpts.EnableExperimentalConcurrentValueChecking &&
+              !isConcurrentValueType(getDeclContext(), indexExpr->getType())) {
+            ctx.Diags.diagnose(
+                component.getLoc(), diag::non_concurrent_keypath_capture,
+                indexExpr->getType());
+          }
+        }
+      }
+
       // The children of #selector expressions are not evaluated, so we do not
       // need to do isolation checking there. This is convenient because such
       // expressions tend to violate restrictions on the use of instance

lib/Sema/TypeCheckConcurrency.h

@@ -66,36 +66,6 @@ enum class ConcurrentReferenceKind {
   ConcurrentFunction,
 };
 
-/// Describes why or where a particular entity has a non-concurrent-value type.
-struct NonConcurrentType {
-  enum Kind {
-    /// A function parameter is a non-concurrent-value type.
-    Parameter,
-    /// The result of a function is a non-concurrent-value type.
-    Result,
-    /// The type of a property is a non-concurrent-value type.
-    Property,
-  } kind;
-
-  /// The declaration reference.
-  ConcreteDeclRef declRef;
-
-  /// The non-concurrent-value type being diagnosed.
-  Type type;
-
-  /// Determine whether a reference to the given declaration involves a
-  /// non-concurrent-value type. If it does, return the reason. Otherwise,
-  /// return \c None.
-  ///
-  /// \param dc The declaration context from which the reference occurs.
-  /// \param declRef The reference to the declaration.
-  static Optional<NonConcurrentType> get(
-      const DeclContext *dc, ConcreteDeclRef declRef);
-
-  /// Diagnose the non-concurrent-value type at the given source location.
-  void diagnose(SourceLoc loc);
-};
-
 /// The isolation restriction in effect for a given declaration that is
 /// referenced from source.
 class ActorIsolationRestriction {

test/Concurrency/concurrent_value_checking.swift

@@ -122,6 +122,23 @@ func testConcurrency() {
   }
 }
 
+// ----------------------------------------------------------------------
+// ConcurrentValue restriction on key paths.
+// ----------------------------------------------------------------------
+class NC: Hashable {
+  func hash(into: inout Hasher) { }
+  static func==(_: NC, _: NC) -> Bool { true }
+}
+
+class HasNC {
+  var dict: [NC: Int] = [:]
+}
+
+func testKeyPaths(dict: [NC: Int], nc: NC) {
+  _ = \HasNC.dict[nc] // expected-warning{{cannot form key path that captures non-concurrent-value type 'NC'}}
+}
+
+
 // ----------------------------------------------------------------------
 // ConcurrentValue restriction on conformances.
 // ----------------------------------------------------------------------