[region-isolation] Change WARNINGS to be ERRORs that are limited as warnings until swift 6.

gottesmm · gottesmm · commit d77dede6cef5 · 2024-01-27T16:16:33.000-08:00
I added some small infrastructure so that the default way one creates
diagnostics sets this automagically so mistakes cannot result.

rdar://121755281
diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def
@@ -890,26 +890,26 @@ NOTE(sil_referencebinding_inout_binding_here, none,
 
 // Warnings arising from the flow-sensitive checking of Sendability of
 // non-Sendable values
-WARNING(regionbasedisolation_selforargtransferred, none,
-        "call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
-WARNING(regionbasedisolation_transfer_yields_race_no_isolation, none,
-        "transferring value of non-Sendable type %0; later accesses could race",
-        (Type))
-WARNING(regionbasedisolation_transfer_yields_race_with_isolation, none,
-        "transferring value of non-Sendable type %0 from %1 context to %2 context; later accesses could race",
-        (Type, ActorIsolation, ActorIsolation))
-WARNING(regionbasedisolation_isolated_capture_yields_race, none,
-        "%1 closure captures value of non-Sendable type %0 from %2 context; later accesses to value could race",
-        (Type, ActorIsolation, ActorIsolation))
-WARNING(regionbasedisolation_transfer_yields_race_transferring_parameter, none,
-        "transferring value of non-Sendable type %0 into transferring parameter; later accesses could race",
-        (Type))
-WARNING(regionbasedisolation_transfer_yields_race_stronglytransferred_binding, none,
-        "binding of non-Sendable type %0 accessed after being transferred; later accesses could race",
-        (Type))
-WARNING(regionbasedisolation_arg_transferred, none,
-        "task isolated value of type %0 transferred to %1 context; later accesses to value could race",
-        (Type, ActorIsolation))
+ERROR(regionbasedisolation_selforargtransferred, none,
+      "call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
+ERROR(regionbasedisolation_transfer_yields_race_no_isolation, none,
+      "transferring value of non-Sendable type %0; later accesses could race",
+      (Type))
+ERROR(regionbasedisolation_transfer_yields_race_with_isolation, none,
+      "transferring value of non-Sendable type %0 from %1 context to %2 context; later accesses could race",
+      (Type, ActorIsolation, ActorIsolation))
+ERROR(regionbasedisolation_isolated_capture_yields_race, none,
+      "%1 closure captures value of non-Sendable type %0 from %2 context; later accesses to value could race",
+      (Type, ActorIsolation, ActorIsolation))
+ERROR(regionbasedisolation_transfer_yields_race_transferring_parameter, none,
+      "transferring value of non-Sendable type %0 into transferring parameter; later accesses could race",
+      (Type))
+ERROR(regionbasedisolation_transfer_yields_race_stronglytransferred_binding, none,
+      "binding of non-Sendable type %0 accessed after being transferred; later accesses could race",
+      (Type))
+ERROR(regionbasedisolation_arg_transferred, none,
+      "task isolated value of type %0 transferred to %1 context; later accesses to value could race",
+      (Type, ActorIsolation))
 NOTE(regionbasedisolation_maybe_race, none,
      "access here could race", ())
 NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
diff --git a/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp b/lib/SILOptimizer/Mandatory/TransferNonSendable.cpp
@@ -56,40 +56,78 @@ using Region = PartitionPrimitives::Region;
 //===----------------------------------------------------------------------===//
 
 template <typename... T, typename... U>
-static InFlightDiagnostic diagnose(ASTContext &context, SourceLoc loc,
-                                   Diag<T...> diag, U &&...args) {
+static InFlightDiagnostic diagnoseError(ASTContext &context, SourceLoc loc,
+                                        Diag<T...> diag, U &&...args) {
+  return std::move(context.Diags.diagnose(loc, diag, std::forward<U>(args)...)
+                       .warnUntilSwiftVersion(6));
+}
+
+template <typename... T, typename... U>
+static InFlightDiagnostic diagnoseError(ASTContext &context, SILLocation loc,
+                                        Diag<T...> diag, U &&...args) {
+  return ::diagnoseError(context, loc.getSourceLoc(), diag,
+                         std::forward<U>(args)...);
+}
+
+template <typename... T, typename... U>
+static InFlightDiagnostic diagnoseError(const PartitionOp &op, Diag<T...> diag,
+                                        U &&...args) {
+  return ::diagnoseError(op.getSourceInst()->getFunction()->getASTContext(),
+                         op.getSourceLoc().getSourceLoc(), diag,
+                         std::forward<U>(args)...);
+}
+
+template <typename... T, typename... U>
+static InFlightDiagnostic diagnoseError(const Operand *op, Diag<T...> diag,
+                                        U &&...args) {
+  return ::diagnoseError(op->getUser()->getFunction()->getASTContext(),
+                         op->getUser()->getLoc().getSourceLoc(), diag,
+                         std::forward<U>(args)...);
+}
+
+template <typename... T, typename... U>
+static InFlightDiagnostic diagnoseError(const SILInstruction *inst,
+                                        Diag<T...> diag, U &&...args) {
+  return ::diagnoseError(inst->getFunction()->getASTContext(),
+                         inst->getLoc().getSourceLoc(), diag,
+                         std::forward<U>(args)...);
+}
+
+template <typename... T, typename... U>
+static InFlightDiagnostic diagnoseNote(ASTContext &context, SourceLoc loc,
+                                       Diag<T...> diag, U &&...args) {
   return context.Diags.diagnose(loc, diag, std::forward<U>(args)...);
 }
 
 template <typename... T, typename... U>
-static InFlightDiagnostic diagnose(ASTContext &context, SILLocation loc,
-                                   Diag<T...> diag, U &&...args) {
-  return ::diagnose(context, loc.getSourceLoc(), diag,
-                    std::forward<U>(args)...);
+static InFlightDiagnostic diagnoseNote(ASTContext &context, SILLocation loc,
+                                       Diag<T...> diag, U &&...args) {
+  return ::diagnoseNote(context, loc.getSourceLoc(), diag,
+                        std::forward<U>(args)...);
 }
 
 template <typename... T, typename... U>
-static InFlightDiagnostic diagnose(const PartitionOp &op, Diag<T...> diag,
-                                   U &&...args) {
-  return ::diagnose(op.getSourceInst()->getFunction()->getASTContext(),
-                    op.getSourceLoc().getSourceLoc(), diag,
-                    std::forward<U>(args)...);
+static InFlightDiagnostic diagnoseNote(const PartitionOp &op, Diag<T...> diag,
+                                       U &&...args) {
+  return ::diagnoseNote(op.getSourceInst()->getFunction()->getASTContext(),
+                        op.getSourceLoc().getSourceLoc(), diag,
+                        std::forward<U>(args)...);
 }
 
 template <typename... T, typename... U>
-static InFlightDiagnostic diagnose(const Operand *op, Diag<T...> diag,
-                                   U &&...args) {
-  return ::diagnose(op->getUser()->getFunction()->getASTContext(),
-                    op->getUser()->getLoc().getSourceLoc(), diag,
-                    std::forward<U>(args)...);
+static InFlightDiagnostic diagnoseNote(const Operand *op, Diag<T...> diag,
+                                       U &&...args) {
+  return ::diagnoseNote(op->getUser()->getFunction()->getASTContext(),
+                        op->getUser()->getLoc().getSourceLoc(), diag,
+                        std::forward<U>(args)...);
 }
 
 template <typename... T, typename... U>
-static InFlightDiagnostic diagnose(const SILInstruction *inst, Diag<T...> diag,
-                                   U &&...args) {
-  return ::diagnose(inst->getFunction()->getASTContext(),
-                    inst->getLoc().getSourceLoc(), diag,
-                    std::forward<U>(args)...);
+static InFlightDiagnostic diagnoseNote(const SILInstruction *inst,
+                                       Diag<T...> diag, U &&...args) {
+  return ::diagnoseNote(inst->getFunction()->getASTContext(),
+                        inst->getLoc().getSourceLoc(), diag,
+                        std::forward<U>(args)...);
 }
 
 //===----------------------------------------------------------------------===//
@@ -944,7 +982,7 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
     // editor UIs providing a more actionable error message.
     auto applyUses = diagnosticInferrer.getApplyUses();
     if (applyUses.empty()) {
-      diagnose(transferOp, diag::regionbasedisolation_unknown_pattern);
+      diagnoseError(transferOp, diag::regionbasedisolation_unknown_pattern);
       continue;
     }
 
@@ -956,29 +994,31 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
         llvm_unreachable("Should never see this!");
       case UseDiagnosticInfoKind::IsolationCrossing: {
         auto isolation = info.diagInfo.getIsolationCrossing();
-        diagnose(astContext, info.loc,
-                 diag::regionbasedisolation_transfer_yields_race_with_isolation,
-                 info.inferredType, isolation.getCallerIsolation(),
-                 isolation.getCalleeIsolation())
+        diagnoseError(
+            astContext, info.loc,
+            diag::regionbasedisolation_transfer_yields_race_with_isolation,
+            info.inferredType, isolation.getCallerIsolation(),
+            isolation.getCalleeIsolation())
             .highlight(info.loc.getSourceRange());
         break;
       }
       case UseDiagnosticInfoKind::RaceWithoutKnownIsolationCrossing:
-        diagnose(astContext, info.loc,
-                 diag::regionbasedisolation_transfer_yields_race_no_isolation,
-                 info.inferredType)
+        diagnoseError(
+            astContext, info.loc,
+            diag::regionbasedisolation_transfer_yields_race_no_isolation,
+            info.inferredType)
             .highlight(info.loc.getSourceRange());
         break;
       case UseDiagnosticInfoKind::UseOfStronglyTransferredValue:
-        diagnose(
+        diagnoseError(
             astContext, info.loc,
             diag::
                 regionbasedisolation_transfer_yields_race_stronglytransferred_binding,
             info.inferredType)
             .highlight(info.loc.getSourceRange());
         break;
       case UseDiagnosticInfoKind::AssignmentIntoTransferringParameter:
-        diagnose(
+        diagnoseError(
             astContext, info.loc,
             diag::
                 regionbasedisolation_transfer_yields_race_transferring_parameter,
@@ -987,10 +1027,10 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
         break;
       case UseDiagnosticInfoKind::IsolationCrossingDueToCapture:
         auto isolation = info.diagInfo.getIsolationCrossing();
-        diagnose(astContext, info.loc,
-                 diag::regionbasedisolation_isolated_capture_yields_race,
-                 info.inferredType, isolation.getCalleeIsolation(),
-                 isolation.getCallerIsolation())
+        diagnoseError(astContext, info.loc,
+                      diag::regionbasedisolation_isolated_capture_yields_race,
+                      info.inferredType, isolation.getCalleeIsolation(),
+                      isolation.getCallerIsolation())
             .highlight(info.loc.getSourceRange());
         break;
       }
@@ -1011,7 +1051,7 @@ void TransferNonSendableImpl::emitUseAfterTransferDiagnostics() {
         if (!liveness.finalRequires.contains(require))
           continue;
 
-        diagnose(require, diag::regionbasedisolation_maybe_race)
+        diagnoseNote(require, diag::regionbasedisolation_maybe_race)
             .highlight(require->getLoc().getSourceRange());
         didEmitRequire = true;
       }
@@ -1041,13 +1081,13 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
     case UseDiagnosticInfoKind::Invalid:
       llvm_unreachable("Should never see this");
     case UseDiagnosticInfoKind::MiscUse:
-      diagnose(astContext, loc,
-               diag::regionbasedisolation_selforargtransferred);
+      diagnoseError(astContext, loc,
+                    diag::regionbasedisolation_selforargtransferred);
       break;
     case UseDiagnosticInfoKind::FunctionArgumentApply: {
-      diagnose(astContext, loc, diag::regionbasedisolation_arg_transferred,
-               op->get()->getType().getASTType(),
-               diagnosticInfo.transferredIsolation.value())
+      diagnoseError(astContext, loc, diag::regionbasedisolation_arg_transferred,
+                    op->get()->getType().getASTType(),
+                    diagnosticInfo.transferredIsolation.value())
           .highlight(op->getUser()->getLoc().getSourceRange());
       // Only emit the note if our value is different from the function
       // argument.
@@ -1057,16 +1097,16 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
       if (rep.maybeGetValue() == info.nonTransferrableValue)
         continue;
       auto *fArg = cast<SILFunctionArgument>(info.nonTransferrableValue);
-      diagnose(
+      diagnoseNote(
           astContext, fArg->getDecl()->getLoc(),
           diag::regionbasedisolation_isolated_since_in_same_region_basename,
           "task isolated", fArg->getDecl()->getBaseName());
       break;
     }
     case UseDiagnosticInfoKind::FunctionArgumentClosure: {
-      diagnose(astContext, loc, diag::regionbasedisolation_arg_transferred,
-               op->get()->getType().getASTType(),
-               diagnosticInfo.transferredIsolation.value())
+      diagnoseError(astContext, loc, diag::regionbasedisolation_arg_transferred,
+                    op->get()->getType().getASTType(),
+                    diagnosticInfo.transferredIsolation.value())
           .highlight(op->getUser()->getLoc().getSourceRange());
       // Only emit the note if our value is different from the function
       // argument.
@@ -1076,7 +1116,7 @@ void TransferNonSendableImpl::emitTransferredNonTransferrableDiagnostics() {
       if (rep.maybeGetValue() == info.nonTransferrableValue)
         continue;
       auto *fArg = cast<SILFunctionArgument>(info.nonTransferrableValue);
-      diagnose(
+      diagnoseNote(
           astContext, fArg->getDecl()->getLoc(),
           diag::regionbasedisolation_isolated_since_in_same_region_basename,
           "task isolated", fArg->getDecl()->getBaseName());
diff --git a/test/Concurrency/transfernonsendable_warning_until_swift6.swift b/test/Concurrency/transfernonsendable_warning_until_swift6.swift
@@ -0,0 +1,30 @@
+// RUN: %target-swift-frontend -emit-sil -strict-concurrency=complete -enable-experimental-feature RegionBasedIsolation -disable-availability-checking -verify %s -o /dev/null -swift-version 6 -enable-experimental-feature TransferringArgsAndResults
+
+// REQUIRES: concurrency
+// REQUIRES: asserts
+
+// This test makes sure that all of our warnings are errors in swift6 mode.
+
+////////////////////////
+// MARK: Declarations //
+////////////////////////
+
+class NonSendableType {}
+
+@MainActor func transferToMain<T>(_ t: T) async {}
+func useValue<T>(_ t: T) {}
+func transferValue<T>(_ t: transferring T) {}
+
+/////////////////
+// MARK: Tests //
+/////////////////
+
+func testIsolationError() async {
+  let x = NonSendableType()
+  await transferToMain(x) // expected-error {{transferring value of non-Sendable type 'NonSendableType' from nonisolated context to main actor-isolated context; later accesses could race}}
+  useValue(x) // expected-note {{access here could race}}
+}
+
+func testArgumentError(_ x: NonSendableType) async { // expected-note {{value is task isolated since it is in the same region as 'x'}}
+  await transferToMain(x) // expected-error {{task isolated value of type 'NonSendableType' transferred to main actor-isolated context; later accesses to value could race}}
+}
