Merge pull request swiftlang#78491 from DougGregor/strict-safety-more-cases

Strict safety more cases

Author: DougGregor

include/swift/AST/DiagnosticsFrontend.def

@@ -589,9 +589,9 @@ ERROR(experimental_not_supported_in_production,none,
       "experimental feature '%0' cannot be enabled in production compiler",
       (StringRef))
 
-GROUPED_WARNING(Ounchecked_with_strict_safety,Unsafe,none,
-      "'-Ounchecked' is not memory-safe and should not be combined with "
-      "strict memory safety checking", ())
+GROUPED_WARNING(command_line_conflicts_with_strict_safety,Unsafe,none,
+      "'%0' is not memory-safe and should not be combined with "
+      "strict memory safety checking", (StringRef))
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/AST/DiagnosticsSema.def

@@ -8097,6 +8097,8 @@ NOTE(note_reference_to_nonisolated_unsafe,none,
      (const ValueDecl *))
 NOTE(note_reference_unowned_unsafe,none,
      "reference to unowned(unsafe) %kind0 is unsafe", (const ValueDecl *))
+NOTE(note_reference_exclusivity_unchecked,none,
+     "reference to @exclusivity(unchecked) %kind0 is unsafe", (const ValueDecl *))
 NOTE(note_use_of_unsafe_conformance_is_unsafe,none,
      "@unsafe conformance of %0 to %kind1 involves unsafe code",
      (Type, const ValueDecl *))
@@ -8118,6 +8120,9 @@ GROUPED_WARNING(use_of_unsafe_conformance_is_unsafe,Unsafe,none,
        (Type, const ValueDecl *))
 GROUPED_WARNING(reference_unowned_unsafe,Unsafe,none,
       "reference to unowned(unsafe) %kind0 is unsafe", (const ValueDecl *))
+GROUPED_WARNING(reference_exclusivity_unchecked,Unsafe,none,
+      "reference to @exclusivity(unchecked) %kind0 is unsafe", (const ValueDecl *))
+
 GROUPED_WARNING(reference_to_nonisolated_unsafe,Unsafe,none,
       "reference to nonisolated(unsafe) %kind0 is unsafe in concurrently-executing code",
        (const ValueDecl *))

include/swift/AST/UnsafeUse.h

@@ -39,6 +39,8 @@ class UnsafeUse {
     UnsafeConformance,
     /// A reference to an unowned(unsafe) entity.
     UnownedUnsafe,
+    /// A reference to an @exclusivity(unchecked) entity.
+    ExclusivityUnchecked,
     /// A reference to a nonisolated(unsafe) entity.
     NonisolatedUnsafe,
     /// A reference to an unsafe declaration.
@@ -111,6 +113,7 @@ class UnsafeUse {
       SourceLoc location
   ) {
     assert(kind == UnownedUnsafe ||
+           kind == ExclusivityUnchecked ||
            kind == NonisolatedUnsafe ||
            kind == ReferenceToUnsafe ||
            kind == ReferenceToUnsafeThroughTypealias ||
@@ -166,6 +169,12 @@ class UnsafeUse {
     return forReference(UnownedUnsafe, dc, decl, Type(), location);
   }
 
+  static UnsafeUse forExclusivityUnchecked(const Decl *decl,
+                                           SourceLoc location,
+                                           DeclContext *dc) {
+    return forReference(ExclusivityUnchecked, dc, decl, Type(), location);
+  }
+
   static UnsafeUse forNonisolatedUnsafe(const Decl *decl,
                                         SourceLoc location,
                                         DeclContext *dc) {
@@ -215,6 +224,7 @@ class UnsafeUse {
             (const char *)storage.typeWitness.location));
 
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
@@ -238,6 +248,7 @@ class UnsafeUse {
       return storage.typeWitness.assocType;
 
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
@@ -262,6 +273,7 @@ class UnsafeUse {
   DeclContext *getDeclContext() const {
     switch (getKind()) {
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
@@ -294,6 +306,7 @@ class UnsafeUse {
 
     case TypeWitness:
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
@@ -319,6 +332,7 @@ class UnsafeUse {
       return storage.typeWitness.type;
 
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
@@ -342,6 +356,7 @@ class UnsafeUse {
 
     case Override:
     case UnownedUnsafe:
+    case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:

lib/Frontend/CompilerInvocation.cpp

@@ -3840,7 +3840,15 @@ bool CompilerInvocation::parseArgs(
   if (LangOpts.hasFeature(Feature::WarnUnsafe)) {
     if (SILOpts.RemoveRuntimeAsserts ||
         SILOpts.AssertConfig == SILOptions::Unchecked) {
-      Diags.diagnose(SourceLoc(), diag::Ounchecked_with_strict_safety);
+      Diags.diagnose(SourceLoc(),
+                     diag::command_line_conflicts_with_strict_safety,
+                     "-Ounchecked");
+    }
+
+    if (!LangOpts.EnableAccessControl) {
+      Diags.diagnose(SourceLoc(),
+                     diag::command_line_conflicts_with_strict_safety,
+                     "-disable-access-control");
     }
   }
 

lib/Sema/TypeCheckAvailability.cpp

@@ -4252,6 +4252,17 @@ diagnoseDeclUnsafe(const ValueDecl *D, SourceRange R,
           return;
         }
       }
+
+      // @exclusivity(unchecked) is unsafe.
+      if (auto exclusivityAttr =
+              var->getAttrs().getAttribute<ExclusivityAttr>()) {
+        if (exclusivityAttr->getMode() == ExclusivityAttr::Unchecked) {
+          diagnoseUnsafeUse(
+              UnsafeUse::forExclusivityUnchecked(var, R.Start,
+                                                 Where.getDeclContext()));
+          return;
+        }
+      }
     }
   }
 }

lib/Sema/TypeCheckUnsafe.cpp

@@ -50,6 +50,7 @@ static bool isUnsafeUseInDefinition(const UnsafeUse &use) {
   case UnsafeUse::CallToUnsafe:
   case UnsafeUse::NonisolatedUnsafe:
   case UnsafeUse::UnownedUnsafe:
+  case UnsafeUse::ExclusivityUnchecked:
   case UnsafeUse::UnsafeConformance:
     return enclosingContextForUnsafe(use).second;
   }
@@ -98,6 +99,7 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use, bool asNote) {
         use.getKind() == UnsafeUse::CallToUnsafe ||
         use.getKind() == UnsafeUse::NonisolatedUnsafe ||
         use.getKind() == UnsafeUse::UnownedUnsafe ||
+        use.getKind() == UnsafeUse::ExclusivityUnchecked ||
         use.getKind() == UnsafeUse::UnsafeConformance) {
       auto [enclosingDecl, _] = enclosingContextForUnsafe(
           use.getLocation(), use.getDeclContext());
@@ -178,6 +180,21 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use, bool asNote) {
     return;
   }
 
+  case UnsafeUse::ExclusivityUnchecked: {
+    auto var = cast<VarDecl>(use.getDecl());
+    ASTContext &ctx = var->getASTContext();
+    ctx.Diags.diagnose(
+        use.getLocation(),
+        asNote ? diag::note_reference_exclusivity_unchecked
+               : diag::reference_exclusivity_unchecked,
+        var);
+    if (!asNote) {
+      var->diagnose(diag::make_enclosing_context_unsafe, var)
+        .fixItInsert(var->getAttributeInsertionLoc(false), "@unsafe ");
+    }
+    return;
+  }
+      
   case UnsafeUse::NonisolatedUnsafe: {
     auto decl = use.getDecl();
     ASTContext &ctx = decl->getASTContext();

test/Unsafe/unsafe.swift

@@ -96,6 +96,19 @@ struct SuperHolder {
   }
 }
 
+// -----------------------------------------------------------------------
+// Dynamic exclusivity check suppression
+// -----------------------------------------------------------------------
+class ExclusivityChecking {
+  @exclusivity(unchecked) var value: Int = 0
+
+  // expected-warning@+1{{instance method 'next' involves unsafe code; use '@safe(unchecked)' to assert that the code is memory-safe}}
+  func next() -> Int {
+    value += 1 // expected-note{{reference to @exclusivity(unchecked) property 'value' is unsafe}}
+    return value  // expected-note{{reference to @exclusivity(unchecked) property 'value' is unsafe}}
+  }
+}
+
 // -----------------------------------------------------------------------
 // Inheritance of @unsafe
 // -----------------------------------------------------------------------

test/Unsafe/unsafe_Ounchecked.swift renamed to test/Unsafe/unsafe_command_line.swift

@@ -1,5 +1,6 @@
-// RUN: %target-swift-frontend -typecheck -enable-experimental-feature WarnUnsafe -Ounchecked %s 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend -typecheck -enable-experimental-feature WarnUnsafe -Ounchecked -disable-access-control %s 2>&1 | %FileCheck %s
 
 // REQUIRES: swift_feature_WarnUnsafe
 
 // CHECK: warning: '-Ounchecked' is not memory-safe
+// CHECK: warning: '-disable-access-control' is not memory-safe