Merge pull request #78187 from DougGregor/suggest-safe-unchecked

Suggest `@safe(unchecked)` when only the body of a declaration uses unsafe constructs

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -8091,14 +8091,17 @@ GROUPED_WARNING(reference_to_unsafe_typed_decl,Unsafe,none,
       (bool, const ValueDecl *, Type))
 NOTE(unsafe_decl_here,none,
      "unsafe %kindbase0 declared here", (const ValueDecl *))
+NOTE(encapsulate_unsafe_in_enclosing_context,none,
+     "make %kindbase0 @safe(unchecked) to allow it to use unsafe constructs in its definition",
+     (const Decl *))
 NOTE(make_enclosing_context_unsafe,none,
-     "mark the enclosing %kindbase0 '@unsafe' to allow it to use unsafe constructs",
+     "make %kindbase0 @unsafe to indicate that its use is not memory-safe",
      (const Decl *))
 NOTE(make_subclass_unsafe,none,
-     "mark the class %0 '@unsafe' to allow unsafe overrides of safe superclass methods",
+     "make class %0 @unsafe to allow unsafe overrides of safe superclass methods",
      (DeclName))
 NOTE(make_conforming_context_unsafe,none,
-     "mark the enclosing %0 with '@unsafe' to allow unsafe conformance to protocol %1",
+     "make the enclosing %0 @unsafe to allow unsafe conformance to protocol %1",
      (DescriptiveDeclKind, DeclName))
 
 //===----------------------------------------------------------------------===//

lib/Sema/TypeCheckAvailability.cpp

@@ -4073,8 +4073,17 @@ static void suggestUnsafeOnEnclosingDecl(
   if (!decl)
     return;
 
-  decl->diagnose(diag::make_enclosing_context_unsafe, decl)
-    .fixItInsert(decl->getAttributeInsertionLoc(false), "@unsafe ");
+  if (versionCheckNode.has_value()) {
+    // The unsafe construct is inside the body of the entity, so suggest
+    // @safe(unchecked) on the declaration.
+    decl->diagnose(diag::encapsulate_unsafe_in_enclosing_context, decl)
+      .fixItInsert(decl->getAttributeInsertionLoc(false),
+                   "@safe(unchecked) ");
+  } else {
+    // The unsafe construct is not part of the body, so
+    decl->diagnose(diag::make_enclosing_context_unsafe, decl)
+      .fixItInsert(decl->getAttributeInsertionLoc(false), "@unsafe ");
+  }
 }
 
 /// Diagnose uses of unavailable declarations. Returns true if a diagnostic

test/Interop/Cxx/class/safe-interop-mode.swift

@@ -55,20 +55,20 @@ struct MyContainer {
 import Test
 import CoreFoundation
 
-// expected-note@+1{{mark the enclosing global function 'useUnsafeParam' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+1{{make global function 'useUnsafeParam' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 func useUnsafeParam(x: Unannotated) { // expected-warning{{reference to unsafe struct 'Unannotated'}}
 }
 
-// expected-note@+2{{mark the enclosing global function 'useUnsafeParam2' '@unsafe' to allow it to use unsafe constructs}}{{10:1-1=@unsafe }}
+// expected-note@+2{{make global function 'useUnsafeParam2' @unsafe to indicate that its use is not memory-safe}}{{10:1-1=@unsafe }}
 @available(SwiftStdlib 5.8, *)
 func useUnsafeParam2(x: UnsafeReference) { // expected-warning{{reference to unsafe class 'UnsafeReference'}}
 }
 
-// expected-note@+1{{mark the enclosing global function 'useUnsafeParam3' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+1{{make global function 'useUnsafeParam3' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 func useUnsafeParam3(x: UnknownEscapabilityAggregate) { // expected-warning{{reference to unsafe struct 'UnknownEscapabilityAggregate'}}
 }
 
-// expected-note@+1{{mark the enclosing global function 'useSafeParams' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+1{{make global function 'useSafeParams' @safe(unchecked) to allow it to use unsafe constructs in its definition}}{{1-1=@safe(unchecked) }}
 func useSafeParams(x: Owner, y: View, z: SafeEscapableAggregate, c: MyContainer) {
     let _ = c.__beginUnsafe() // expected-warning{{call to unsafe instance method '__beginUnsafe'}}
 }

test/Unsafe/safe.swift

@@ -19,7 +19,7 @@ func g() {
   unsafeFunction()
 }
 
-// expected-note@+2{{mark the enclosing global function 'h' '@unsafe' to allow it to use unsafe constructs}}
+// expected-note@+2{{make global function 'h' @unsafe to indicate that its use is not memory-safe}}
 @safe(unchecked, message: "I was careful")
 func h(_: UnsafeType) { // expected-warning{{reference to unsafe struct 'UnsafeType' [Unsafe]}}
   unsafeFunction()

test/Unsafe/unsafe-suppression.swift

@@ -14,7 +14,7 @@ struct UnsafeType { } // expected-note{{unsafe struct 'UnsafeType' declared here
 
 // expected-warning@+1{{reference to unsafe struct 'UnsafeType' [Unsafe]}}
 func iAmImpliedUnsafe() -> UnsafeType? { nil }
-// expected-note@-1{{mark the enclosing global function 'iAmImpliedUnsafe' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@-1{{make global function 'iAmImpliedUnsafe' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 
 @unsafe
 func labeledUnsafe(_: UnsafeType) {
@@ -27,7 +27,7 @@ class C {
   func method() { } // expected-note{{overridden declaration is here}}
 }
 
-class D1: C { // expected-note{{mark the class 'D1' '@unsafe' to allow unsafe overrides of safe superclass methods}}{{1-1=@unsafe }}
+class D1: C { // expected-note{{make class 'D1' @unsafe to allow unsafe overrides of safe superclass methods}}{{1-1=@unsafe }}
   @unsafe
   override func method() { } // expected-warning{{override of safe instance method with unsafe instance method [Unsafe]}}
 }
@@ -42,7 +42,7 @@ protocol P {
 }
 
 struct S1: P {
-  // expected-note@-1{{mark the enclosing struct with '@unsafe' to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
+  // expected-note@-1{{make the enclosing struct @unsafe to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
   @unsafe
   func protoMethod() { } // expected-warning{{unsafe instance method 'protoMethod()' cannot satisfy safe requirement}}
 }
@@ -56,7 +56,7 @@ struct S2: P {
 struct S3 { }
 
 extension S3: P {
-  // expected-note@-1{{mark the enclosing extension with '@unsafe' to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
+  // expected-note@-1{{make the enclosing extension @unsafe to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
   @unsafe
   func protoMethod() { } // expected-warning{{unsafe instance method 'protoMethod()' cannot satisfy safe requirement}}
 }
@@ -100,7 +100,7 @@ struct UnsafeOuter { // expected-note{{unsafe struct 'UnsafeOuter' declared here
   }
 }
 
-// expected-note@+1{{mark the enclosing extension of struct 'UnsafeOuter' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+1{{make extension of struct 'UnsafeOuter' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 extension UnsafeOuter { // expected-warning{{reference to unsafe struct 'UnsafeOuter' [Unsafe]}}
   func h(_: UnsafeType) { }
 }

test/Unsafe/unsafe.swift

@@ -20,7 +20,7 @@ protocol P {
 }
 
 struct XP: P {
-  // expected-note@-1{{mark the enclosing struct with '@unsafe' to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
+  // expected-note@-1{{make the enclosing struct @unsafe to allow unsafe conformance to protocol 'P'}}{{1-1=@unsafe }}
   @unsafe func f() { } // expected-warning{{unsafe instance method 'f()' cannot satisfy safe requirement [Unsafe]}}
   @unsafe func g() { }
 }
@@ -34,7 +34,7 @@ protocol Ptrable2 {
 }
 
 extension HasAPointerType: Ptrable2 { } // expected-warning{{unsafe type 'HasAPointerType.Ptr' (aka 'PointerType') cannot satisfy safe associated type 'Ptr'}}
-  // expected-note@-1{{mark the enclosing extension with '@unsafe' to allow unsafe conformance to protocol 'Ptrable2'}}{{1-1=@unsafe }}
+  // expected-note@-1{{make the enclosing extension @unsafe to allow unsafe conformance to protocol 'Ptrable2'}}{{1-1=@unsafe }}
 
 // -----------------------------------------------------------------------
 // Overrides
@@ -44,7 +44,7 @@ class Super {
   @unsafe func g() { }
 }
 
-class Sub: Super { // expected-note{{mark the class 'Sub' '@unsafe' to allow unsafe overrides of safe superclass methods}}{{1-1=@unsafe }}
+class Sub: Super { // expected-note{{make class 'Sub' @unsafe to allow unsafe overrides of safe superclass methods}}{{1-1=@unsafe }}
   @unsafe override func f() { } // expected-warning{{override of safe instance method with unsafe instance method [Unsafe]}}
   @unsafe override func g() { }  
 }
@@ -55,7 +55,7 @@ class Sub: Super { // expected-note{{mark the class 'Sub' '@unsafe' to allow uns
 struct SuperHolder {
   unowned var s1: Super
   unowned(unsafe) var s2: Super // expected-warning{{unowned(unsafe) involves unsafe code}}
-  // expected-note@-1{{mark the enclosing property 's2' '@unsafe' to allow it to use unsafe constructs}}{{3-3=@unsafe }}
+  // expected-note@-1{{make property 's2' @unsafe to indicate that its use is not memory-safe}}{{3-3=@unsafe }}
 }
 
 // -----------------------------------------------------------------------
@@ -66,15 +66,16 @@ struct SuperHolder {
 };
 
 class UnsafeSub: UnsafeSuper { } // expected-warning{{reference to unsafe class 'UnsafeSuper'}}
-// expected-note@-1{{mark the enclosing class 'UnsafeSub' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@-1{{make class 'UnsafeSub' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 
 // -----------------------------------------------------------------------
 // Declaration references
 // -----------------------------------------------------------------------
 @unsafe func unsafeF() { } // expected-note{{unsafe global function 'unsafeF' declared here}}
 @unsafe var unsafeVar: Int = 0 // expected-note{{'unsafeVar' declared here}}
 
-// expected-note@+1 7{{mark the enclosing global function 'testMe' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+2 5{{make global function 'testMe' @safe(unchecked) to allow it to use unsafe constructs in its definition}}{{1-1=@safe(unchecked) }}
+// expected-note@+1 2{{make global function 'testMe' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 func testMe(
   _ pointer: PointerType, // expected-warning{{reference to unsafe struct 'PointerType'}}
   _ unsafeSuper: UnsafeSuper // expected-warning{{reference to unsafe class 'UnsafeSuper'}}

test/Unsafe/unsafe_concurrency.swift

@@ -6,15 +6,15 @@
 // REQUIRES: swift_feature_WarnUnsafe
 
 // expected-warning@+2{{@unchecked conformance involves unsafe code}}
-// expected-note@+1{{mark the enclosing class with '@unsafe' to allow unsafe conformance to protocol 'Sendable'}}{{1-1=@unsafe }}
+// expected-note@+1{{make the enclosing class @unsafe to allow unsafe conformance to protocol 'Sendable'}}{{1-1=@unsafe }}
 class C: @unchecked Sendable {
   var counter: Int = 0
 }
 
 @available(SwiftStdlib 5.1, *)
 func f() async {
   // expected-warning@+2{{nonisolated(unsafe) involves unsafe code}}
-  // expected-note@+1{{mark the enclosing var 'counter' '@unsafe' to allow it to use unsafe constructs}}
+  // expected-note@+1{{make var 'counter' @unsafe to indicate that its use is not memory-safe}}
   nonisolated(unsafe) var counter = 0
   Task.detached {
     counter += 1

test/Unsafe/unsafe_imports.swift

@@ -5,7 +5,8 @@
 
 import unsafe_decls
 
-// expected-note@+1 3{{mark the enclosing global function 'testUnsafe' '@unsafe' to allow it to use unsafe constructs}}
+// expected-note@+2 2{{make global function 'testUnsafe' @safe(unchecked) to allow it to use unsafe constructs in its definition}}
+// expected-note@+1{{make global function 'testUnsafe' @unsafe to indicate that its use is not memory-safe}}
 func testUnsafe(_ ut: UnsafeType) { // expected-warning{{reference to unsafe struct 'UnsafeType'}}
   unsafe_c_function() // expected-warning{{call to unsafe global function 'unsafe_c_function'}}
 

test/Unsafe/unsafe_stdlib.swift

@@ -6,7 +6,8 @@
 // REQUIRES: swift_feature_AllowUnsafeAttribute
 // REQUIRES: swift_feature_WarnUnsafe
 
-// expected-note@+1 4{{mark the enclosing global function 'test' '@unsafe' to allow it to use unsafe constructs}}{{1-1=@unsafe }}
+// expected-note@+2 2{{make global function 'test' @safe(unchecked) to allow it to use unsafe constructs in its definition}}{{1-1=@safe(unchecked) }}
+// expected-note@+1 2{{make global function 'test' @unsafe to indicate that its use is not memory-safe}}{{1-1=@unsafe }}
 func test(
   x: OpaquePointer, // expected-warning{{reference to unsafe struct 'OpaquePointer'}}
   other: UnsafeMutablePointer<Int> // expected-warning{{reference to unsafe generic struct 'UnsafeMutablePointer'}}