Diagnose references to unowned(unsafe) variables as unsafe uses

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -8084,6 +8084,8 @@ NOTE(note_reference_to_unsafe_typed_decl,none,
 NOTE(note_reference_to_nonisolated_unsafe,none,
      "reference to nonisolated(unsafe) %kind0 is unsafe in concurrently-executing code",
      (const ValueDecl *))
+NOTE(note_reference_unowned_unsafe,none,
+     "reference to unowned(unsafe) %kind0 is unsafe", (const ValueDecl *))
 
 GROUPED_WARNING(override_safe_withunsafe,Unsafe,none,
       "override of safe %0 with unsafe %0", (DescriptiveDeclKind))
@@ -8095,8 +8097,8 @@ GROUPED_WARNING(type_witness_unsafe,Unsafe,none,
       (Type, DeclName))
 GROUPED_WARNING(unchecked_conformance_is_unsafe,Unsafe,none,
       "@unchecked conformance involves unsafe code", ())
-GROUPED_WARNING(unowned_unsafe_is_unsafe,Unsafe,none,
-      "unowned(unsafe) involves unsafe code", ())
+GROUPED_WARNING(reference_unowned_unsafe,Unsafe,none,
+      "reference to unowned(unsafe) %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 *))

lib/Sema/TypeCheckAttr.cpp

@@ -42,7 +42,6 @@
 #include "swift/AST/SwiftNameTranslation.h"
 #include "swift/AST/TypeCheckRequests.h"
 #include "swift/AST/Types.h"
-#include "swift/AST/UnsafeUse.h"
 #include "swift/Basic/Assertions.h"
 #include "swift/Parse/Lexer.h"
 #include "swift/Parse/ParseDeclName.h"
@@ -4976,15 +4975,6 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type,
     }
   }
 
-  // unowned(unsafe) is unsafe (duh).
-  if (ownershipKind == ReferenceOwnership::Unmanaged &&
-      ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&
-      !var->allowsUnsafe()) {
-    diagnoseUnsafeUse(
-        UnsafeUse::forUnownedUnsafe(var, attr->getLocation(),
-                                    var->getDeclContext()));
-  }
-
   if (attr->isInvalid())
     return type;
 

lib/Sema/TypeCheckAvailability.cpp

@@ -4229,6 +4229,19 @@ diagnoseDeclUnsafe(const ValueDecl *D, SourceRange R,
           valueDecl, R.Start, Where.getDeclContext()));
       return;
     }
+
+    if (auto var = dyn_cast<VarDecl>(valueDecl)) {
+      // unowned(unsafe) is unsafe (duh).
+      if (auto ownershipAttr =
+              var->getAttrs().getAttribute<ReferenceOwnershipAttr>()) {
+        if (ownershipAttr->get() == ReferenceOwnership::Unmanaged) {
+          diagnoseUnsafeUse(
+              UnsafeUse::forUnownedUnsafe(var, R.Start,
+                                          Where.getDeclContext()));
+          return;
+        }
+      }
+    }
   }
 }
 

lib/Sema/TypeCheckUnsafe.cpp

@@ -39,13 +39,13 @@ static bool isUnsafeUseInDefinition(const UnsafeUse &use) {
   case UnsafeUse::Witness:
   case UnsafeUse::TypeWitness:
   case UnsafeUse::UnsafeConformance:
-  case UnsafeUse::UnownedUnsafe:
     // Never part of the definition. These are always part of the interface.
     return false;
 
   case UnsafeUse::ReferenceToUnsafe:
   case UnsafeUse::CallToUnsafe:
   case UnsafeUse::NonisolatedUnsafe:
+  case UnsafeUse::UnownedUnsafe:
     return enclosingContextForUnsafe(use).second;
   }
 }
@@ -100,7 +100,8 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use, bool asNote) {
     // same declaration.
     if (use.getKind() == UnsafeUse::ReferenceToUnsafe ||
         use.getKind() == UnsafeUse::CallToUnsafe ||
-        use.getKind() == UnsafeUse::NonisolatedUnsafe) {
+        use.getKind() == UnsafeUse::NonisolatedUnsafe ||
+        use.getKind() == UnsafeUse::UnownedUnsafe) {
       auto [enclosingDecl, _] = enclosingContextForUnsafe(
           use.getLocation(), use.getDeclContext());
       if (enclosingDecl) {
@@ -164,9 +165,16 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use, bool asNote) {
 
   case UnsafeUse::UnownedUnsafe: {
     auto var = cast<VarDecl>(use.getDecl());
-    var->diagnose(diag::unowned_unsafe_is_unsafe);
-    var->diagnose(diag::make_enclosing_context_unsafe, var)
-      .fixItInsert(var->getAttributeInsertionLoc(false), "@unsafe ");
+    ASTContext &ctx = var->getASTContext();
+    ctx.Diags.diagnose(
+        use.getLocation(),
+        asNote ? diag::note_reference_unowned_unsafe
+               : diag::reference_unowned_unsafe,
+        var);
+    if (!asNote) {
+      var->diagnose(diag::make_enclosing_context_unsafe, var)
+        .fixItInsert(var->getAttributeInsertionLoc(false), "@unsafe ");
+    }
     return;
   }
 

test/Unsafe/unsafe.swift

@@ -54,8 +54,18 @@ class Sub: Super { // expected-note{{make class 'Sub' @unsafe to allow unsafe ov
 // -----------------------------------------------------------------------
 struct SuperHolder {
   unowned var s1: Super
-  unowned(unsafe) var s2: Super // expected-warning{{unowned(unsafe) involves unsafe code}}
-  // expected-note@-1{{make property 's2' @unsafe to indicate that its use is not memory-safe}}{{3-3=@unsafe }}
+  unowned(unsafe) var s2: Super
+
+  // expected-warning@+1{{instance method 'getSuper2' involves unsafe code; use '@safe(unchecked)' to assert that the code is memory-safe}}
+  func getSuper2() -> Super {
+    return s2 // expected-note{{reference to unowned(unsafe) property 's2' is unsafe}}
+  }
+
+  // FIXME: We should be able to identify this as being inside the function
+  // expected-warning@+1{{instance method 'getSuper2b' involves unsafe code; use '@unsafe' to indicate that its use is not memory-safe}}
+  func getSuper2b() -> Super {
+    s2 // expected-note{{reference to unowned(unsafe) property 's2' is unsafe}}
+  }
 }
 
 // -----------------------------------------------------------------------