[Concurrency] Diagnose concurrency violations in derived conformances at valid

hborla · hborla · commit 85cced7a147e · 2023-12-20T06:55:10.000-08:00
source locations.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -5559,6 +5559,9 @@ ERROR(isolated_parameter_combined_nonisolated,none,
       "%0 with 'isolated' parameter cannot be 'nonisolated'",
       (DescriptiveDeclKind))
 
+NOTE(in_derived_conformance, none,
+     "in derived conformance to %0",
+     (Type))
 WARNING(non_sendable_param_type,none,
         "non-sendable type %0 %select{passed in call to %3 %kind2|"
         "exiting %3 context in call to non-isolated %kind2|"
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -1002,11 +1002,13 @@ void swift::diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf) {
 /// Produce a diagnostic for a single instance of a non-Sendable type where
 /// a Sendable type is required.
 static bool diagnoseSingleNonSendableType(
-    Type type, SendableCheckContext fromContext, SourceLoc loc,
+    Type type, SendableCheckContext fromContext,
+    Type inDerivedConformance, SourceLoc loc,
     llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
 
   auto module = fromContext.fromDC->getParentModule();
   auto nominal = type->getAnyNominal();
+  auto &ctx = module->getASTContext();
 
   return diagnoseSendabilityErrorBasedOn(nominal, fromContext,
                                          [&](DiagnosticBehavior behavior) {
@@ -1017,9 +1019,13 @@ static bool diagnoseSingleNonSendableType(
     if (wasSuppressed || behavior == DiagnosticBehavior::Ignore)
       return true;
 
+    if (inDerivedConformance) {
+      ctx.Diags.diagnose(loc, diag::in_derived_conformance,
+                         inDerivedConformance);
+    }
+
     if (type->is<FunctionType>()) {
-      module->getASTContext().Diags
-          .diagnose(loc, diag::nonsendable_function_type);
+      ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
     } else if (nominal && nominal->getParentModule() == module) {
       // If the nominal type is in the current module, suggest adding
       // `Sendable` if it might make sense. Otherwise, just complain.
@@ -1052,7 +1058,8 @@ static bool diagnoseSingleNonSendableType(
 }
 
 bool swift::diagnoseNonSendableTypes(
-    Type type, SendableCheckContext fromContext, SourceLoc loc,
+    Type type, SendableCheckContext fromContext,
+    Type inDerivedConformance, SourceLoc loc,
     llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
   auto module = fromContext.fromDC->getParentModule();
 
@@ -1064,15 +1071,17 @@ bool swift::diagnoseNonSendableTypes(
   // FIXME: More detail for unavailable conformances.
   auto conformance = TypeChecker::conformsToProtocol(type, proto, module);
   if (conformance.isInvalid() || conformance.hasUnavailableConformance()) {
-    return diagnoseSingleNonSendableType(type, fromContext, loc, diagnose);
+    return diagnoseSingleNonSendableType(
+        type, fromContext, inDerivedConformance, loc, diagnose);
   }
 
   // Walk the conformance, diagnosing any missing Sendable conformances.
   bool anyMissing = false;
   conformance.forEachMissingConformance(module,
       [&](BuiltinProtocolConformance *missing) {
         if (diagnoseSingleNonSendableType(
-                missing->getType(), fromContext, loc, diagnose)) {
+                missing->getType(), fromContext,
+                inDerivedConformance, loc, diagnose)) {
           anyMissing = true;
         }
 
@@ -1095,11 +1104,27 @@ bool swift::diagnoseNonSendableTypesInReference(
     return swift::getActorIsolation(declRef.getDecl());
   };
 
+  // If the violation is in the implementation of a derived conformance,
+  // point to the location of the parent type instead.
+  Type derivedConformanceType;
+  if (refLoc.isInvalid()) {
+    auto *decl = fromDC->getAsDecl();
+    if (decl && decl->isImplicit()) {
+      if (auto *implements = decl->getAttrs().getAttribute<ImplementsAttr>()) {
+        auto *parentDC = decl->getDeclContext();
+        refLoc = parentDC->getAsDecl()->getLoc();
+        derivedConformanceType =
+            implements->getProtocol(parentDC)->getDeclaredInterfaceType();
+      }
+    }
+  }
+
   // Check the 'self' argument.
   if (base) {
     if (diagnoseNonSendableTypes(
             base->getType(),
-            fromDC, base->getStartLoc(),
+            fromDC, derivedConformanceType,
+            base->getStartLoc(),
             diag::non_sendable_param_type,
             (unsigned)refKind, declRef.getDecl(),
             getActorIsolation()))
@@ -1114,7 +1139,8 @@ bool swift::diagnoseNonSendableTypesInReference(
       for (auto param : *function->getParameters()) {
         Type paramType = param->getInterfaceType().subst(subs);
         if (diagnoseNonSendableTypes(
-            paramType, fromDC, refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
+                paramType, fromDC, derivedConformanceType,
+                refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
                 diag::non_sendable_param_type,
             (unsigned)refKind, function, getActorIsolation()))
           return true;
@@ -1127,7 +1153,8 @@ bool swift::diagnoseNonSendableTypesInReference(
         // only check results if funcCheckKind specifies so
         Type resultType = func->getResultInterfaceType().subst(subs);
         if (diagnoseNonSendableTypes(
-            resultType, fromDC, refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
+            resultType, fromDC, derivedConformanceType,
+            refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
                 diag::non_sendable_result_type,
             (unsigned)refKind, func, getActorIsolation()))
           return true;
@@ -1142,7 +1169,8 @@ bool swift::diagnoseNonSendableTypesInReference(
         ? var->getTypeInContext()
         : var->getValueInterfaceType().subst(subs);
     if (diagnoseNonSendableTypes(
-            propertyType, fromDC, refLoc,
+            propertyType, fromDC,
+            derivedConformanceType, refLoc,
             diag::non_sendable_property_type,
             var,
             var->isLocalCapture(),
@@ -1157,7 +1185,8 @@ bool swift::diagnoseNonSendableTypesInReference(
         // Check params of this subscript override for sendability
         Type paramType = param->getInterfaceType().subst(subs);
         if (diagnoseNonSendableTypes(
-                paramType, fromDC, refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
+                paramType, fromDC, derivedConformanceType,
+                refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
                 diag::non_sendable_param_type,
                 (unsigned)refKind, subscript, getActorIsolation()))
           return true;
@@ -1168,7 +1197,8 @@ bool swift::diagnoseNonSendableTypesInReference(
       // Check the element type of a subscript.
       Type resultType = subscript->getElementInterfaceType().subst(subs);
       if (diagnoseNonSendableTypes(
-          resultType, fromDC, refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
+              resultType, fromDC, derivedConformanceType,
+              refLoc, diagnoseLoc.isInvalid() ? refLoc : diagnoseLoc,
               diag::non_sendable_result_type,
           (unsigned)refKind, subscript, getActorIsolation()))
         return true;
@@ -1183,7 +1213,8 @@ bool swift::diagnoseNonSendableTypesInReference(
 void swift::diagnoseMissingSendableConformance(
     SourceLoc loc, Type type, const DeclContext *fromDC) {
   diagnoseNonSendableTypes(
-      type, fromDC, loc, diag::non_sendable_type);
+      type, fromDC, /*inDerivedConformance*/Type(),
+      loc, diag::non_sendable_type);
 }
 
 namespace {
@@ -1935,7 +1966,8 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
     auto *base = selfApply->getBase();
     if (diagnoseNonSendableTypes(
             base->getType(),
-            declContext, base->getStartLoc(),
+            declContext, /*inDerivedConformance*/Type(),
+            base->getStartLoc(),
             diag::non_sendable_call_argument,
             isolationCrossing.value().exitsIsolation(),
             isolationCrossing.value().getDiagnoseIsolation()))
@@ -1976,7 +2008,8 @@ bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *dec
 
     if (diagnoseNonSendableTypes(
             argType ? argType : param.getParameterType(),
-            declContext, argLoc, diag::non_sendable_call_argument,
+            declContext, /*inDerivedConformance*/Type(),
+            argLoc, diag::non_sendable_call_argument,
             isolationCrossing.value().exitsIsolation(),
             isolationCrossing.value().getDiagnoseIsolation()))
       return true;
@@ -2418,19 +2451,24 @@ namespace {
             // into async let to SIL level region based isolation.
             if (!ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation)) {
               diagnoseNonSendableTypes(
-                  type, getDeclContext(), capture.getLoc(),
+                  type, getDeclContext(),
+                  /*inDerivedConformance*/Type(), capture.getLoc(),
                   diag::implicit_async_let_non_sendable_capture,
                   decl->getName());
             }
           } else {
             // Fallback to a generic implicit capture missing sendable
             // conformance diagnostic.
-            diagnoseNonSendableTypes(type, getDeclContext(), capture.getLoc(),
+            diagnoseNonSendableTypes(type, getDeclContext(),
+                                     /*inDerivedConformance*/Type(),
+                                     capture.getLoc(),
                                      diag::implicit_non_sendable_capture,
                                      decl->getName());
           }
         } else {
-          diagnoseNonSendableTypes(type, getDeclContext(), capture.getLoc(),
+          diagnoseNonSendableTypes(type, getDeclContext(),
+                                   /*inDerivedConformance*/Type(),
+                                   capture.getLoc(),
                                    diag::non_sendable_capture, decl->getName(),
                                    /*closure=*/closure != nullptr);
         }
@@ -3341,7 +3379,9 @@ namespace {
 
       // Check for sendability of the result type.
       if (diagnoseNonSendableTypes(
-             fnType->getResult(), getDeclContext(), apply->getLoc(),
+             fnType->getResult(), getDeclContext(),
+             /*inDerivedConformance*/Type(),
+             apply->getLoc(),
              diag::non_sendable_call_result_type,
              apply->isImplicitlyAsync().has_value(),
              *unsatisfiedIsolation))
@@ -3483,6 +3523,7 @@ namespace {
                                          llvm::None)) {
               if (diagnoseNonSendableTypes(
                              component.getComponentType(), getDeclContext(),
+                             /*inDerivedConformance*/Type(),
                              component.getLoc(),
                              diag::non_sendable_keypath_access)) {
                 diagnosed = true;
@@ -3516,6 +3557,7 @@ namespace {
               auto type = getType(arg.getExpr());
               if (type && shouldDiagnoseExistingDataRaces(getDeclContext()) &&
                   diagnoseNonSendableTypes(type, getDeclContext(),
+                                           /*inDerivedConformance*/Type(),
                                            component.getLoc(),
                                            diag::non_sendable_keypath_capture))
                 diagnosed = true;
@@ -5163,7 +5205,8 @@ static bool checkSendableInstanceStorage(
 
       // Check that the property type is Sendable.
       diagnoseNonSendableTypes(
-          propertyType, SendableCheckContext(dc, check), property->getLoc(),
+          propertyType, SendableCheckContext(dc, check),
+          /*inDerivedConformance*/Type(), property->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
             if (isImplicitSendableCheck(check)) {
               // If this is for an externally-visible conformance, fail.
@@ -5199,7 +5242,8 @@ static bool checkSendableInstanceStorage(
     /// Handle an enum associated value.
     bool operator()(EnumElementDecl *element, Type elementType) override {
       diagnoseNonSendableTypes(
-          elementType, SendableCheckContext(dc, check), element->getLoc(),
+          elementType, SendableCheckContext(dc, check),
+          /*inDerivedConformance*/Type(), element->getLoc(),
           [&](Type type, DiagnosticBehavior behavior) {
             if (isImplicitSendableCheck(check)) {
               // If this is for an externally-visible conformance, fail.
diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h
@@ -393,7 +393,8 @@ struct SendableCheckContext {
 /// \returns \c true if any errors were produced, \c false if no diagnostics or
 /// only warnings and notes were produced.
 bool diagnoseNonSendableTypes(
-    Type type, SendableCheckContext fromContext, SourceLoc loc,
+    Type type, SendableCheckContext fromContext,
+    Type inDerivedConformance, SourceLoc loc,
     llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose);
 
 namespace detail {
@@ -416,14 +417,15 @@ namespace detail {
 template<typename ...DiagArgs>
 bool diagnoseNonSendableTypes(
     Type type, SendableCheckContext fromContext,
+    Type derivedConformance,
     SourceLoc typeLoc, SourceLoc diagnoseLoc,
     Diag<Type, DiagArgs...> diag,
     typename detail::Identity<DiagArgs>::type ...diagArgs) {
 
     ASTContext &ctx = fromContext.fromDC->getASTContext();
     return diagnoseNonSendableTypes(
-        type, fromContext, typeLoc, [&](Type specificType,
-                                        DiagnosticBehavior behavior) {
+        type, fromContext, derivedConformance, typeLoc,
+        [&](Type specificType, DiagnosticBehavior behavior) {
 
           if (behavior != DiagnosticBehavior::Ignore) {
             ctx.Diags.diagnose(diagnoseLoc, diag, type, diagArgs...)
@@ -441,12 +443,14 @@ bool diagnoseNonSendableTypes(
 /// only warnings and notes were produced.
 template<typename ...DiagArgs>
 bool diagnoseNonSendableTypes(
-    Type type, SendableCheckContext fromContext, SourceLoc loc,
+    Type type, SendableCheckContext fromContext,
+    Type derivedConformance, SourceLoc loc,
     Diag<Type, DiagArgs...> diag,
     typename detail::Identity<DiagArgs>::type ...diagArgs) {
 
-    return diagnoseNonSendableTypes(type, fromContext, loc, loc, diag,
-                             std::forward<decltype(diagArgs)>(diagArgs)...);
+    return diagnoseNonSendableTypes(
+        type, fromContext, derivedConformance, loc, loc, diag,
+        std::forward<decltype(diagArgs)>(diagArgs)...);
 }
 
 /// Diagnose this sendability error with behavior based on the import of
diff --git a/test/Concurrency/sendable_checking.swift b/test/Concurrency/sendable_checking.swift
@@ -344,3 +344,17 @@ func testPointersAreNotSendable() {
     testSendable(rawMutableBuffer.makeIterator()) // expected-warning {{conformance of 'UnsafeRawBufferPointer.Iterator' to 'Sendable' is unavailable}}
   }
 }
+
+@available(*, unavailable)
+extension SynthesizedConformances.NotSendable: Sendable {}
+
+enum SynthesizedConformances {
+  // expected-note@+1 2 {{consider making struct 'NotSendable' conform to the 'Sendable' protocol}}
+  struct NotSendable: Equatable {}
+
+  // expected-warning@+2 2{{non-sendable type 'SynthesizedConformances.NotSendable' in asynchronous access to main actor-isolated property 'x' cannot cross actor boundary}}
+  // expected-note@+1 2 {{in derived conformance to 'Equatable'}}
+  @MainActor struct Isolated: Equatable {
+    let x: NotSendable
+  }
+}
