Merge pull request #85090 from kavon/manual-ownership/usability-fixes-6

ManualOwnership: provide ability to apply to entire compilation unit

Author: kavon

include/swift/AST/DeclAttr.def

@@ -815,7 +815,7 @@ SIMPLE_DECL_ATTR(_noObjCBridging, NoObjCBridging,
   UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   155)
 
-SIMPLE_DECL_ATTR(_manualOwnership, ManualOwnership,
+SIMPLE_DECL_ATTR(_noManualOwnership, NoManualOwnership,
   OnAbstractFunction | OnSubscript,
   UserInaccessible | ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove | ForbiddenInABIAttr,
   156)

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")
@@ -73,6 +74,7 @@ GROUP(ProtocolTypeNonConformance, "protocol-type-non-conformance")
 GROUP(RegionIsolation, "region-isolation")
 GROUP(ResultBuilderMethods, "result-builder-methods")
 GROUP(ReturnTypeImplicitCopy, "return-type-implicit-copy")
+GROUP(SemanticCopies, "semantic-copies")
 GROUP(SendableClosureCaptures, "sendable-closure-captures")
 GROUP(SendableMetatypes, "sendable-metatypes")
 GROUP(SendingClosureRisksDataRace, "sending-closure-risks-data-race")

include/swift/AST/DiagnosticsSIL.def

@@ -433,15 +433,22 @@ 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", ())
-ERROR(manualownership_copy,none,
-      "explicit 'copy' required here; please report this vague diagnostic as a bug", ())
-ERROR(manualownership_copy_happened,none,
+
+// ManualOwnership diagnostics
+GROUPED_WARNING(manualownership_copy,SemanticCopies,DefaultIgnore,
+      "implicit 'copy' happens here; please report this vague diagnostic as a bug", ())
+GROUPED_WARNING(manualownership_copy_happened,SemanticCopies,DefaultIgnore,
       "accessing %0 may produce a copy; write 'copy' to acknowledge or 'consume' to elide", (Identifier))
-ERROR(manualownership_copy_demanded,none,
+GROUPED_WARNING(manualownership_copy_demanded,SemanticCopies,DefaultIgnore,
       "independent copy of %0 is required here; write 'copy' to acknowledge or 'consume' to elide", (Identifier))
-ERROR(manualownership_copy_captured,none,
+GROUPED_WARNING(manualownership_copy_captured,SemanticCopies,DefaultIgnore,
       "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,DefaultIgnore,
+      "exclusive access here will be checked at runtime; please report this vague diagnostic as a bug", ())
+GROUPED_WARNING(manualownership_exclusivity_named,DynamicExclusivity,DefaultIgnore,
+      "accessing %0 here may incur runtime exclusivity check%1", (Identifier, StringRef))
+
 // 'transparent' diagnostics
 ERROR(circular_transparent,none,
       "inlining 'transparent' functions forms circular loop", ())

include/swift/AST/DiagnosticsSema.def

@@ -2145,9 +2145,7 @@ ERROR(attr_static_exclusive_only_mutating,none,
 ERROR(attr_static_exclusive_no_setters,none,
       "variable of type %0 must not have a setter", (Type))
 
-// @_manualOwnership
-ERROR(attr_manual_ownership_experimental,none,
-      "'@_manualOwnership' requires '-enable-experimental-feature ManualOwnership'", ())
+// ManualOwnership
 ERROR(attr_manual_ownership_noimplicitcopy,none,
       "'@_noImplicitCopy' cannot be used with ManualOwnership", ())
 

include/swift/Basic/Features.def

@@ -449,7 +449,9 @@ EXPERIMENTAL_FEATURE(LifetimeDependence, true)
 /// Enable the `@_staticExclusiveOnly` attribute.
 EXPERIMENTAL_FEATURE(StaticExclusiveOnly, true)
 
-/// Enable the `@_manualOwnership` attribute.
+/// Enable the ManualOwnership diagnostic groups:
+///   * -Wwarning SemanticCopies
+///   * -Wwarning DynamicExclusivity
 EXPERIMENTAL_FEATURE(ManualOwnership, false)
 
 /// Enable the @extractConstantsFromMembers attribute.

lib/AST/ASTDumper.cpp

@@ -5024,7 +5024,7 @@ class PrintAttribute : public AttributeVisitor<PrintAttribute, void, Label>,
   TRIVIAL_ATTR_PRINTER(NoLocks, no_locks)
   TRIVIAL_ATTR_PRINTER(NoMetadata, no_metadata)
   TRIVIAL_ATTR_PRINTER(NoObjCBridging, no_objc_bridging)
-  TRIVIAL_ATTR_PRINTER(ManualOwnership, manual_ownership)
+  TRIVIAL_ATTR_PRINTER(NoManualOwnership, no_manual_ownership)
   TRIVIAL_ATTR_PRINTER(NoRuntime, no_runtime)
   TRIVIAL_ATTR_PRINTER(NonEphemeral, non_ephemeral)
   TRIVIAL_ATTR_PRINTER(NonEscapable, non_escapable)

lib/ASTGen/Sources/ASTGen/DeclAttrs.swift

@@ -265,7 +265,6 @@ extension ASTGenVisitor {
         .LexicalLifetimes,
         .LLDBDebuggerFunction,
         .MainType,
-        .ManualOwnership,
         .Marker,
         .MoveOnly,
         .NeverEmitIntoClient,
@@ -276,6 +275,7 @@ extension ASTGenVisitor {
         .NoRuntime,
         .NoImplicitCopy,
         .NoLocks,
+        .NoManualOwnership,
         .NoMetadata,
         .NoObjCBridging,
         .NonEphemeral,

lib/SIL/IR/SILFunctionBuilder.cpp

@@ -208,7 +208,9 @@ void SILFunctionBuilder::addFunctionAttributes(
     F->setPerfConstraints(PerformanceConstraints::NoExistentials);
   } else if (Attrs.hasAttribute<NoObjCBridgingAttr>()) {
     F->setPerfConstraints(PerformanceConstraints::NoObjCBridging);
-  } else if (Attrs.hasAttribute<ManualOwnershipAttr>()) {
+  } else if (M.getASTContext().LangOpts.hasFeature(Feature::ManualOwnership) &&
+             constant && constant.hasDecl() && !constant.isImplicit() &&
+             !Attrs.hasAttribute<NoManualOwnershipAttr>()) {
     F->setPerfConstraints(PerformanceConstraints::ManualOwnership);
   }
 

lib/SILGen/SILGen.cpp

@@ -1368,15 +1368,15 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F,
   //
   // If ManualOwnership ends up subsuming those prior mechanisms for an
   // explicit-copy mode, we can move this somewhere else, like postEmitFunction.
-  if (auto *ace = constant.getAbstractClosureExpr()) {
-    if (auto *dc = ace->getOutermostFunctionContext()) {
-      if (auto *decl = dc->getAsDecl()) {
-        if (decl->getAttrs().hasAttribute<ManualOwnershipAttr>()) {
-          F->setPerfConstraints(PerformanceConstraints::ManualOwnership);
-        }
-      }
-    }
-  }
+  //
+  // FIXME: maybe this should happen in SILFunctionBuilder::getOrCreateFunction
+  if (getASTContext().LangOpts.hasFeature(Feature::ManualOwnership))
+    if (auto *ace = constant.getAbstractClosureExpr())
+      if (auto *dc = ace->getOutermostFunctionContext())
+        if (auto *decl = dc->getAsDecl())
+          if (!decl->isImplicit() &&
+              !decl->getAttrs().hasAttribute<NoManualOwnershipAttr>())
+            F->setPerfConstraints(PerformanceConstraints::ManualOwnership);
 
   LLVM_DEBUG(llvm::dbgs() << "lowering ";
              F->printName(llvm::dbgs());

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;
   }