Merge pull request #73216 from gottesmm/release/6.0-region-iso

[6.0][region-isolation] Fix import pre concurrency support for region isolation

Author: gottesmm

include/swift/SIL/SILFunction.h

@@ -269,6 +269,9 @@ class SILFunction
   /// @_dynamicReplacement(for:) function.
   SILFunction *ReplacedFunction = nullptr;
 
+  /// The SILDeclRef that this function was created for by SILGen if one exists.
+  SILDeclRef DeclRef = SILDeclRef();
+
   /// This SILFunction REFerences an ad-hoc protocol requirement witness in
   /// order to keep it alive, such that it main be obtained in IRGen. Without
   /// this explicit reference, the witness would seem not-used, and not be
@@ -1114,6 +1117,12 @@ class SILFunction
   /// Get the source location of the function.
   const SILDebugScope *getDebugScope() const { return DebugScope; }
 
+  /// Return the SILDeclRef for this SILFunction if one was assigned by SILGen.
+  SILDeclRef getDeclRef() const { return DeclRef; }
+
+  /// Set the SILDeclRef for this SILFunction. Used mainly by SILGen.
+  void setDeclRef(SILDeclRef declRef) { DeclRef = declRef; }
+
   /// Get this function's bare attribute.
   IsBare_t isBare() const { return IsBare_t(Bare); }
   void setBare(IsBare_t isB) { Bare = isB; }
@@ -1376,6 +1385,9 @@ class SILFunction
 
   ActorIsolation getActorIsolation() const { return actorIsolation; }
 
+  /// Return the source file that this SILFunction belongs to if it exists.
+  SourceFile *getSourceFile() const;
+
   //===--------------------------------------------------------------------===//
   // Block List Access
   //===--------------------------------------------------------------------===//

include/swift/SILOptimizer/PassManager/Passes.def

@@ -501,6 +501,8 @@ PASS(ReferenceBindingTransform, "sil-reference-binding-transform",
      "Check/transform reference bindings")
 PASS(DiagnosticDeadFunctionElimination, "sil-diagnostic-dead-function-elim",
      "Eliminate dead functions from early specialization optimizations before we run later diagnostics")
+PASS(DiagnoseUnnecessaryPreconcurrencyImports, "sil-diagnose-unnecessary-preconcurrency-imports",
+     "Diagnose any preconcurrency imports that Sema and TransferNonSendable did not use")
 PASS(PruneVTables, "prune-vtables",
      "Mark class methods that do not require vtable dispatch")
 PASS_RANGE(AllPasses, AADumper, PruneVTables)

include/swift/Sema/Concurrency.h

@@ -0,0 +1,40 @@
+//===--- Concurrency.h ----------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+///
+/// This file defines concurrency utility routines from Sema that are used by
+/// later parts of the compiler like the pass pipeline.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_SEMA_CONCURRENCY_H
+#define SWIFT_SEMA_CONCURRENCY_H
+
+namespace swift {
+
+class SourceFile;
+class NominalTypeDecl;
+
+/// If any of the imports in this source file was @preconcurrency but there were
+/// no diagnostics downgraded or suppressed due to that @preconcurrency, suggest
+/// that the attribute be removed.
+void diagnoseUnnecessaryPreconcurrencyImports(SourceFile &sf);
+
+/// Determine whether the given nominal type has an explicit Sendable
+/// conformance (regardless of its availability).
+bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
+                                    bool applyModuleDefault = true);
+
+} // namespace swift
+
+#endif

lib/SIL/IR/SILFunction.cpp

@@ -1189,3 +1189,11 @@ bool SILFunction::argumentMayRead(Operand *argOp, SILValue addr) {
 
   return argumentMayReadFunction({this}, {argOp}, {addr});
 }
+
+SourceFile *SILFunction::getSourceFile() const {
+  auto declRef = getDeclRef();
+  if (!declRef)
+    return nullptr;
+
+  return declRef.getInnermostDeclContext()->getParentSourceFile();
+}

lib/SILGen/SILGen.cpp

@@ -1236,6 +1236,9 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
   // Create a debug scope for the function using astNode as source location.
   F->setDebugScope(new (M) SILDebugScope(Loc, F));
 
+  // Initialize F with the constant we created for it.
+  F->setDeclRef(constant);
+
   LLVM_DEBUG(llvm::dbgs() << "lowering ";
              F->printName(llvm::dbgs());
              llvm::dbgs() << " : ";

lib/SILOptimizer/Mandatory/CMakeLists.txt

@@ -9,6 +9,7 @@ target_sources(swiftSILOptimizer PRIVATE
   ConsumeOperatorCopyableValuesChecker.cpp
   PhiStorageOptimizer.cpp
   ConstantPropagation.cpp
+  DiagnoseUnnecessaryPreconcurrencyImports.cpp
   DebugInfoCanonicalizer.cpp
   DefiniteInitialization.cpp
   DIMemoryUseCollector.cpp

lib/SILOptimizer/Mandatory/DiagnoseUnnecessaryPreconcurrencyImports.cpp

@@ -0,0 +1,79 @@
+//===--- DiagnoseUnnecessaryPreconcurrencyImports.cpp ---------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+///
+/// This is run after TransferNonSendable and uses Sema infrastructure to
+/// determine if in Sema or TransferNonSendable any of the preconcurrency import
+/// statements were not used.
+///
+/// This only runs when RegionIsolation is enabled. If RegionIsolation is
+/// disabled, we emit the unnecessary preconcurrency imports earlier during Sema
+/// since no later diagnostics will be emitted.
+///
+/// NOTE: This needs to be a module pass and run after TransferNonSendable so we
+/// can guarantee that we have run TransferNonSendable on all functions in our
+/// module before this runs.
+///
+//===----------------------------------------------------------------------===//
+
+#include "swift/AST/SourceFile.h"
+#include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/Sema/Concurrency.h"
+
+using namespace swift;
+
+//===----------------------------------------------------------------------===//
+//                         MARK: Top Level Entrypoint
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class DiagnoseUnnecessaryPreconcurrencyImports : public SILModuleTransform {
+  void run() override {
+    // If region isolation is not enabled... return early.
+    if (!getModule()->getASTContext().LangOpts.hasFeature(
+            Feature::RegionBasedIsolation))
+      return;
+
+    std::vector<SourceFile *> data;
+    for (auto &fn : *getModule()) {
+      auto *sf = fn.getSourceFile();
+      if (!sf) {
+        continue;
+      }
+
+      data.push_back(sf);
+      assert(sf->getBufferID() != -1 && "Must have a buffer id");
+    }
+
+    // Sort unique by filename so our diagnostics are deterministic.
+    //
+    // TODO: If we cannot rely upon this, just sort by pointer address. Non
+    // determinism emission of diagnostics isn't great but it isn't fatal.
+    sortUnique(data, [](SourceFile *lhs, SourceFile *rhs) -> bool {
+      return lhs->getBufferID() < rhs->getBufferID();
+    });
+
+    // At this point, we know that we have our sorted unique list of source
+    // files.
+    for (auto *sf : data) {
+      diagnoseUnnecessaryPreconcurrencyImports(*sf);
+    }
+  }
+};
+
+} // namespace
+
+SILTransform *swift::createDiagnoseUnnecessaryPreconcurrencyImports() {
+  return new DiagnoseUnnecessaryPreconcurrencyImports();
+}

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -15,6 +15,8 @@
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/ProtocolConformance.h"
+#include "swift/AST/SourceFile.h"
 #include "swift/AST/Type.h"
 #include "swift/Basic/FrozenMultiMap.h"
 #include "swift/Basic/ImmutablePointerSet.h"
@@ -35,6 +37,7 @@
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/PartitionUtils.h"
 #include "swift/SILOptimizer/Utils/VariableNameUtils.h"
+#include "swift/Sema/Concurrency.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Debug.h"
 
@@ -55,6 +58,33 @@ using Region = PartitionPrimitives::Region;
 //                              MARK: Utilities
 //===----------------------------------------------------------------------===//
 
+static std::optional<DiagnosticBehavior>
+getDiagnosticBehaviorLimitForValue(SILValue value) {
+  auto *nom = value->getType().getNominalOrBoundGenericNominal();
+  if (!nom)
+    return {};
+
+  auto declRef = value->getFunction()->getDeclRef();
+  if (!declRef)
+    return {};
+
+  auto *fromDC = declRef.getInnermostDeclContext();
+  auto attributedImport = nom->findImport(fromDC);
+  if (!attributedImport ||
+      !attributedImport->options.contains(ImportFlags::Preconcurrency))
+    return {};
+
+  if (auto *sourceFile = fromDC->getParentSourceFile())
+    sourceFile->setImportUsedPreconcurrency(*attributedImport);
+
+  if (hasExplicitSendableConformance(nom))
+    return DiagnosticBehavior::Warning;
+
+  return attributedImport->module.importedModule->isConcurrencyChecked()
+             ? DiagnosticBehavior::Warning
+             : DiagnosticBehavior::Ignore;
+}
+
 static Expr *inferArgumentExprFromApplyExpr(ApplyExpr *sourceApply,
                                             FullApplySite fai,
                                             const Operand *op) {
@@ -438,14 +468,19 @@ class UseAfterTransferDiagnosticEmitter {
       emitUnknownPatternError();
   }
 
+  std::optional<DiagnosticBehavior> getBehaviorLimit() const {
+    return getDiagnosticBehaviorLimitForValue(transferOp->get());
+  }
+
   void
   emitNamedIsolationCrossingError(SILLocation loc, Identifier name,
                                   SILIsolationInfo namedValuesIsolationInfo,
                                   ApplyIsolationCrossing isolationCrossing) {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     // Then emit the note with greater context.
     SmallString<64> descriptiveKindStr;
@@ -466,7 +501,8 @@ class UseAfterTransferDiagnosticEmitter {
         loc, diag::regionbasedisolation_transfer_yields_race_with_isolation,
         inferredType, isolationCrossing.getCallerIsolation(),
         isolationCrossing.getCalleeIsolation())
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -475,7 +511,8 @@ class UseAfterTransferDiagnosticEmitter {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     // Then emit the note with greater context.
     diagnoseNote(
@@ -493,7 +530,8 @@ class UseAfterTransferDiagnosticEmitter {
         diag::
             regionbasedisolation_transfer_yields_race_stronglytransferred_binding,
         inferredType)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -502,7 +540,8 @@ class UseAfterTransferDiagnosticEmitter {
     diagnoseError(loc,
                   diag::regionbasedisolation_transfer_yields_race_no_isolation,
                   inferredType)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
@@ -513,7 +552,8 @@ class UseAfterTransferDiagnosticEmitter {
     // Emit the short error.
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
 
     SmallString<64> descriptiveKindStr;
     {
@@ -535,13 +575,15 @@ class UseAfterTransferDiagnosticEmitter {
     diagnoseError(loc, diag::regionbasedisolation_isolated_capture_yields_race,
                   inferredType, isolationCrossing.getCalleeIsolation(),
                   isolationCrossing.getCallerIsolation())
-        .highlight(loc.getSourceRange());
+        .highlight(loc.getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
     emitRequireInstDiagnostics();
   }
 
   void emitUnknownPatternError() {
     diagnoseError(transferOp->getUser(),
-                  diag::regionbasedisolation_unknown_pattern);
+                  diag::regionbasedisolation_unknown_pattern)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
 private:
@@ -941,20 +983,26 @@ class TransferNonTransferrableDiagnosticEmitter {
     return info.nonTransferrable.dyn_cast<SILInstruction *>();
   }
 
+  std::optional<DiagnosticBehavior> getBehaviorLimit() const {
+    return getDiagnosticBehaviorLimitForValue(info.transferredOperand->get());
+  }
+
   /// Return the isolation region info for \p getNonTransferrableValue().
   SILIsolationInfo getIsolationRegionInfo() const {
     return info.isolationRegionInfo;
   }
 
   void emitUnknownPatternError() {
     diagnoseError(getOperand()->getUser(),
-                  diag::regionbasedisolation_unknown_pattern);
+                  diag::regionbasedisolation_unknown_pattern)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitUnknownUse(SILLocation loc) {
     // TODO: This will eventually be an unknown pattern error.
-    diagnoseError(
-        loc, diag::regionbasedisolation_task_or_actor_isolated_transferred);
+    diagnoseError(loc,
+                  diag::regionbasedisolation_task_or_actor_isolated_transferred)
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitFunctionArgumentApply(SILLocation loc, Type type,
@@ -967,7 +1015,8 @@ class TransferNonTransferrableDiagnosticEmitter {
     diagnoseError(loc, diag::regionbasedisolation_arg_transferred,
                   StringRef(descriptiveKindStr), type,
                   crossing.getCalleeIsolation())
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedFunctionArgumentClosure(SILLocation loc, Identifier name,
@@ -995,13 +1044,15 @@ class TransferNonTransferrableDiagnosticEmitter {
     auto diag =
         diag::regionbasedisolation_arg_passed_to_strongly_transferred_param;
     diagnoseError(loc, diag, descriptiveKindStr, type)
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedOnlyError(SILLocation loc, Identifier name) {
     diagnoseError(loc, diag::regionbasedisolation_named_transfer_yields_race,
                   name)
-        .highlight(getOperand()->getUser()->getLoc().getSourceRange());
+        .highlight(getOperand()->getUser()->getLoc().getSourceRange())
+        .limitBehaviorIf(getBehaviorLimit());
   }
 
   void emitNamedIsolation(SILLocation loc, Identifier name,
@@ -1176,7 +1227,7 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
     if (!isolation) {
       // Otherwise, emit a "we don't know error" that tells the user to file a
       // bug.
-      diagnoseError(op->getUser(), diag::regionbasedisolation_unknown_pattern);
+      diagnosticEmitter.emitUnknownPatternError();
       return false;
     }
     assert(isolation && "Expected non-null");