Warn about an unused @_predatesConcurrency attribute on an import.

We'll already suggest the addition of `@_predatesConcurrency` on an import
to downgrade or silence `Sendable`-related diagnostics, so we'll be
adding these to people's code. Over time, this attribute can go stale,
if the imported module's adopt `Sendable`. When the attribute isn't
doing anything, because it didn't suppress any `Sendable`-related
diagnostics, suggest that it be removed.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -1956,6 +1956,8 @@ REMARK(add_predates_concurrency_import,none,
      "add '@_predatesConcurrency' to %select{suppress|treat}0 "
      "'Sendable'-related %select{warnings|errors}0 from module %1"
      "%select{| as warnings}0", (bool, Identifier))
+REMARK(remove_predates_concurrency_import,none,
+     "'@_predatesConcurrency' attribute on module %0 is unused", (Identifier))
 WARNING(public_decl_needs_sendable,none,
         "public %0 %1 does not specify whether it is 'Sendable' or not",
         (DescriptiveDeclKind, DeclName))

include/swift/AST/Import.h

@@ -556,9 +556,11 @@ struct AttributedImport {
 
   AttributedImport(ModuleInfo module, SourceLoc importLoc = SourceLoc(),
                    ImportOptions options = ImportOptions(),
-                   StringRef filename = {}, ArrayRef<Identifier> spiGroups = {})
+                   StringRef filename = {}, ArrayRef<Identifier> spiGroups = {},
+                   SourceRange predatesConcurrencyRange = {})
       : module(module), importLoc(importLoc), options(options),
-        sourceFileArg(filename), spiGroups(spiGroups) {
+        sourceFileArg(filename), spiGroups(spiGroups),
+        predatesConcurrencyRange(predatesConcurrencyRange) {
     assert(!(options.contains(ImportFlags::Exported) &&
              options.contains(ImportFlags::ImplementationOnly)) ||
            options.contains(ImportFlags::Reserved));
@@ -567,7 +569,8 @@ struct AttributedImport {
   template<class OtherModuleInfo>
   AttributedImport(ModuleInfo module, AttributedImport<OtherModuleInfo> other)
     : AttributedImport(module, other.importLoc, other.options,
-                       other.sourceFileArg, other.spiGroups) { }
+                       other.sourceFileArg, other.spiGroups,
+                       other.predatesConcurrencyRange) { }
 
   friend bool operator==(const AttributedImport<ModuleInfo> &lhs,
                          const AttributedImport<ModuleInfo> &rhs) {

lib/Sema/TypeCheckConcurrency.cpp

@@ -752,6 +752,14 @@ static bool diagnoseSingleNonSendableType(
 
   bool wasSuppressed = diagnose(type, behavior);
 
+  // If this type was imported from another module, try to find the
+  // corresponding import.
+  Optional<AttributedImport<swift::ImportedModule>> import;
+  SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
+  if (nominal && nominal->getParentModule() != module) {
+    import = findImportFor(nominal, fromContext.fromDC);
+  }
+
   if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
     // Don't emit any other diagnostics.
   } else if (type->is<FunctionType>()) {
@@ -775,12 +783,10 @@ static bool diagnoseSingleNonSendableType(
         diag::non_sendable_nominal, nominal->getDescriptiveKind(),
         nominal->getName());
 
-    // If we can find an import in this context that makes this nominal
-    // type visible, remark that it can be `@_predatesConcurrency` import.
+    // If we found the import that makes this nominal type visible, remark
+    // that it can be @_predatesConcurrency import.
     // Only emit this remark once per source file, because it can happen a
     // lot.
-    SourceFile *sourceFile = fromContext.fromDC->getParentSourceFile();
-    auto import = findImportFor(nominal, fromContext.fromDC);
     if (import && !import->options.contains(ImportFlags::PredatesConcurrency) &&
         import->importLoc.isValid() && sourceFile &&
         !sourceFile->hasImportUsedPredatesConcurrency(*import)) {
@@ -795,6 +801,14 @@ static bool diagnoseSingleNonSendableType(
     }
   }
 
+  // If we found an import that makes this nominal type visible, and that
+  // was a @_predatesConcurrency import, note that we have made use of the
+  // attribute.
+  if (import && import->options.contains(ImportFlags::PredatesConcurrency) &&
+      sourceFile) {
+    sourceFile->setImportUsedPredatesConcurrency(*import);
+  }
+
   return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
 }
 

lib/Sema/TypeChecker.cpp

@@ -264,6 +264,23 @@ void swift::performTypeChecking(SourceFile &SF) {
                                  TypeCheckSourceFileRequest{&SF}, {});
 }
 
+/// If any of the imports in this source file was @_predatesConcurrency but
+/// there were no diagnostics downgraded or suppressed due to that
+/// @_predatesConcurrency, suggest that the attribute be removed.
+static void diagnoseUnnecessaryPredatesConcurrencyImports(SourceFile &sf) {
+  ASTContext &ctx = sf.getASTContext();
+  for (const auto &import : sf.getImports()) {
+    if (import.options.contains(ImportFlags::PredatesConcurrency) &&
+        import.importLoc.isValid() &&
+        !sf.hasImportUsedPredatesConcurrency(import)) {
+      ctx.Diags.diagnose(
+          import.importLoc, diag::remove_predates_concurrency_import,
+          import.module.importedModule->getName())
+        .fixItRemove(import.predatesConcurrencyRange);
+    }
+  }
+}
+
 evaluator::SideEffect
 TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
   assert(SF && "Source file cannot be null!");
@@ -305,6 +322,8 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
     typeCheckDelayedFunctions(*SF);
   }
 
+  diagnoseUnnecessaryPredatesConcurrencyImports(*SF);
+
   // Check to see if there's any inconsistent @_implementationOnly imports.
   evaluateOrDefault(
       Ctx.evaluator,

test/Concurrency/predates_concurrency_import.swift

@@ -1,16 +1,24 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module -emit-module-path %t/StrictModule.swiftmodule -module-name StrictModule -warn-concurrency %S/Inputs/StrictModule.swift
 // RUN: %target-swift-frontend -emit-module -emit-module-path %t/NonStrictModule.swiftmodule -module-name NonStrictModule %S/Inputs/NonStrictModule.swift
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking
 
-// RUN: %target-typecheck-verify-swift -typecheck  -I %t %s
+// RUN: %target-typecheck-verify-swift -typecheck  -I %t
 
 @_predatesConcurrency import NonStrictModule
 @_predatesConcurrency import StrictModule
+@_predatesConcurrency import OtherActors
+// expected-remark@-1{{'@_predatesConcurrency' attribute on module 'OtherActors' is unused}}{{1-23=}}
 
 func acceptSendable<T: Sendable>(_: T) { }
 
 @available(SwiftStdlib 5.1, *)
-func test(ss: StrictStruct, ns: NonStrictClass) async {
+func test(
+  ss: StrictStruct, ns: NonStrictClass, oma: OtherModuleActor,
+  ssc: SomeSendableClass
+) async {
   acceptSendable(ss) // expected-warning{{type 'StrictStruct' does not conform to the 'Sendable' protocol}}
   acceptSendable(ns) // silence issue entirely
+  acceptSendable(oma) // okay
+  acceptSendable(ssc) // okay
 }