ManualOwnership: introduce 'DynamicExclusivity'

We can add warnings about dynamic exclusivity
checks that may happen on an access, with
explainers about why they happen for safety.

Author: kavon

include/swift/AST/DiagnosticGroups.def

@@ -49,6 +49,7 @@ GROUP(ConformanceIsolation, "conformance-isolation")
 GROUP(ForeignReferenceType, "foreign-reference-type")
 GROUP(DeprecatedDeclaration, "deprecated-declaration")
 GROUP(DynamicCallable, "dynamic-callable-requirements")
+GROUP(DynamicExclusivity, "dynamic-exclusivity")
 GROUP(EmbeddedRestrictions, "embedded-restrictions")
 GROUP(ErrorInFutureSwiftVersion, "error-in-future-swift-version")
 GROUP(ExclusivityViolation, "exclusivity-violation")

include/swift/AST/DiagnosticsSIL.def

@@ -433,6 +433,8 @@ ERROR(wrong_linkage_for_serialized_function,none,
       "function has wrong linkage to be called from %0", (StringRef))
 NOTE(performance_called_from,none,
       "called from here", ())
+
+// ManualOwnership diagnostics
 GROUPED_WARNING(manualownership_copy,SemanticCopies,none,
       "implicit 'copy' happens here; please report this vague diagnostic as a bug", ())
 GROUPED_WARNING(manualownership_copy_happened,SemanticCopies,none,
@@ -441,6 +443,10 @@ GROUPED_WARNING(manualownership_copy_demanded,SemanticCopies,none,
       "independent copy of %0 is required here; write 'copy' to acknowledge or 'consume' to elide", (Identifier))
 GROUPED_WARNING(manualownership_copy_captured,SemanticCopies,none,
       "closure capture of '%0' requires independent copy of it; write [%0 = copy %0] in the closure's capture list to acknowledge", (StringRef))
+GROUPED_WARNING(manualownership_exclusivity,DynamicExclusivity,none,
+      "exclusive access here will be checked at runtime", ())
+GROUPED_WARNING(manualownership_exclusivity_named,DynamicExclusivity,none,
+      "accessing %0 here may incur runtime exclusivity check%1", (Identifier, StringRef))
 
 // 'transparent' diagnostics
 ERROR(circular_transparent,none,

lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp

@@ -489,6 +489,49 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
         return false;
       }
     }
+    if (impact & RuntimeEffect::ExclusivityChecking) {
+      switch (inst->getKind()) {
+      case SILInstructionKind::BeginUnpairedAccessInst:
+      case SILInstructionKind::EndUnpairedAccessInst:
+        // These instructions are quite unusual; they seem to only ever created
+        // explicitly by calling functions from the Builtin module, see:
+        //   - emitBuiltinPerformInstantaneousReadAccess
+        //   - emitBuiltinEndUnpairedAccess
+        break;
+      case SILInstructionKind::EndAccessInst:
+        break; // We'll already diagnose the begin access.
+      case SILInstructionKind::BeginAccessInst: {
+        auto bai = cast<BeginAccessInst>(inst);
+        auto info = VariableNameInferrer::inferNameAndRoot(bai->getSource());
+
+        if (!info) {
+          LLVM_DEBUG(llvm::dbgs() << "exclusivity (no name?): " << *inst);
+          diagnose(loc, diag::manualownership_exclusivity);
+          return false;
+        }
+        Identifier name = info->first;
+        SILValue root = info->second;
+        StringRef advice = "";
+
+        // Try to classify the root to give advice.
+        if (isa<GlobalAddrInst>(root)) {
+          advice = ", because it involves a global variable";
+        } else if (root->getType().isAnyClassReferenceType()) {
+          advice = ", because it's a member of a reference type";
+        }
+
+        LLVM_DEBUG(llvm::dbgs() << "exclusivity: " << *inst);
+        LLVM_DEBUG(llvm::dbgs() << "with root: " << root);
+
+        diagnose(loc, diag::manualownership_exclusivity_named, name, advice);
+        break;
+      }
+      default:
+        LLVM_DEBUG(llvm::dbgs() << "UNKNOWN EXCLUSIVITY INST: " << *inst);
+        diagnose(loc, diag::manualownership_exclusivity);
+        return false;
+      }
+    }
     return false;
   }
 

lib/SILOptimizer/Utils/VariableNameUtils.cpp

@@ -651,6 +651,7 @@ SILValue VariableNameInferrer::findDebugInfoProvidingValueHelper(
         isa<LoadBorrowInst>(searchValue) || isa<BeginAccessInst>(searchValue) ||
         isa<MarkUnresolvedNonCopyableValueInst>(searchValue) ||
         isa<ProjectBoxInst>(searchValue) || isa<CopyValueInst>(searchValue) ||
+        isa<ExplicitCopyValueInst>(searchValue) ||
         isa<ConvertFunctionInst>(searchValue) ||
         isa<MarkUninitializedInst>(searchValue) ||
         isa<MarkDependenceInst>(searchValue) ||

test/SIL/manual_ownership.swift

@@ -1,5 +1,6 @@
 // RUN: %target-swift-frontend %s -emit-sil -verify \
 // RUN:   -Werror SemanticCopies \
+// RUN:   -Werror DynamicExclusivity \
 // RUN:   -enable-experimental-feature ManualOwnership
 
 // REQUIRES: swift_feature_ManualOwnership
@@ -225,19 +226,19 @@ func reassignments_1_fixed_2() {
 
 @_manualOwnership
 public func basic_loop_trivial_values(_ t: Triangle, _ xs: [Triangle]) {
-  var p: Pair = t.a
+  var p: Pair = t.a // expected-error {{accessing 't.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
   for x in xs { // expected-error {{independent copy of 'xs' is required}}
-    p = p.midpoint(x.a)
+    p = p.midpoint(x.a) // expected-error {{accessing 'x.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
   }
-  t.a = p
+  t.a = p // expected-error {{accessing 't.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
 }
 @_manualOwnership
 public func basic_loop_trivial_values_fixed(_ t: Triangle, _ xs: [Triangle]) {
-  var p: Pair = t.a
+  var p: Pair = t.a // expected-error {{accessing 't.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
   for x in copy xs {
-    p = p.midpoint(x.a)
+    p = p.midpoint(x.a) // expected-error {{accessing 'x.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
   }
-  t.a = p
+  t.a = p // expected-error {{accessing 't.a' here may incur runtime exclusivity check, because it's a member of a reference type}}
 }
 
 // FIXME: the only reason for so many copies below is because

userdocs/diagnostics/dynamic-exclusivity.md

@@ -0,0 +1,3 @@
+# Dynamic Exclusivity (Experimental Diagnostics)
+
+TODO explain