[Strict safety] Diagnose types with unsafe storage

When a type definition involves unsafe types in its storage, require it
to be explicitly marked @unsafe or @safe.

Author: DougGregor

include/swift/AST/DiagnosticsSema.def

@@ -8149,6 +8149,18 @@ NOTE(note_reference_exclusivity_unchecked,none,
 NOTE(note_use_of_unsafe_conformance_is_unsafe,none,
      "@unsafe conformance of %0 to %kind1 involves unsafe code",
      (Type, const ValueDecl *))
+NOTE(note_unsafe_storage,none,
+     "%kindbase1 involves unsafe type %2", (bool, const ValueDecl *, Type))
+
+GROUPED_WARNING(decl_unsafe_storage,Unsafe,none,
+      "%kindbase0 has storage involving unsafe types",
+      (const Decl *))
+NOTE(decl_storage_mark_unsafe,none,
+      "add '@unsafe' if this type is also unsafe to use",
+      ())
+NOTE(decl_storage_mark_safe,none,
+      "add '@safe' if this type encapsulates the unsafe storage in "
+      "a safe interface", ())
 
 GROUPED_WARNING(conformance_involves_unsafe,Unsafe,none,
      "conformance of %0 to %kind1 involves unsafe code; use '@unsafe' to "

include/swift/AST/UnsafeUse.h

@@ -45,6 +45,8 @@ class UnsafeUse {
     NonisolatedUnsafe,
     /// A reference to an unsafe declaration.
     ReferenceToUnsafe,
+    /// A reference to an unsafe storage.
+    ReferenceToUnsafeStorage,
     /// A reference to a typealias that is not itself unsafe, but has
     /// an unsafe underlying type.
     ReferenceToUnsafeThroughTypealias,
@@ -113,6 +115,7 @@ class UnsafeUse {
            kind == ExclusivityUnchecked ||
            kind == NonisolatedUnsafe ||
            kind == ReferenceToUnsafe ||
+           kind == ReferenceToUnsafeStorage ||
            kind == ReferenceToUnsafeThroughTypealias ||
            kind == CallToUnsafe);
 
@@ -179,6 +182,12 @@ class UnsafeUse {
                         decl, type, location);
   }
 
+  static UnsafeUse forReferenceToUnsafeStorage(const Decl *decl,
+                                               Type type,
+                                               SourceLoc location) {
+    return forReference(ReferenceToUnsafeStorage, decl, type, location);
+  }
+
   static UnsafeUse forReferenceToUnsafeThroughTypealias(const Decl *decl,
                                         Type type,
                                         SourceLoc location) {
@@ -215,6 +224,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return SourceLoc(
@@ -246,6 +256,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       storage.entity.location = loc.getOpaquePointerValue();
@@ -266,6 +277,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return storage.entity.decl;
@@ -299,6 +311,7 @@ class UnsafeUse {
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
     case ReferenceToUnsafeThroughTypealias:
+    case ReferenceToUnsafeStorage:
     case CallToUnsafe:
     case UnsafeConformance:
     case PreconcurrencyImport:
@@ -324,6 +337,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
       return storage.entity.type;
@@ -348,6 +362,7 @@ class UnsafeUse {
     case ExclusivityUnchecked:
     case NonisolatedUnsafe:
     case ReferenceToUnsafe:
+    case ReferenceToUnsafeStorage:
     case ReferenceToUnsafeThroughTypealias:
     case CallToUnsafe:
     case PreconcurrencyImport:

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2387,12 +2387,19 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
                           "`" + VD->getBaseName().userFacingName().str() + "`");
       }
 
-      // Expand extension macros.
       if (auto *nominal = dyn_cast<NominalTypeDecl>(VD)) {
+        // Expand extension macros.
         (void)evaluateOrDefault(
             Ctx.evaluator,
             ExpandExtensionMacros{nominal},
             { });
+
+        // If strict memory safety checking is enabled, check the storage
+        // of the nominal type.
+        if (Ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&
+            !isa<ProtocolDecl>(nominal)) {
+          checkUnsafeStorage(nominal);
+        }
       }
     }
   }

lib/Sema/TypeCheckUnsafe.cpp

@@ -16,6 +16,7 @@
 
 #include "TypeCheckAvailability.h"
 #include "TypeCheckConcurrency.h"
+#include "TypeCheckInvertible.h"
 #include "TypeCheckType.h"
 #include "TypeCheckUnsafe.h"
 
@@ -121,6 +122,7 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use) {
   }
 
   case UnsafeUse::ReferenceToUnsafe:
+  case UnsafeUse::ReferenceToUnsafeStorage:
   case UnsafeUse::CallToUnsafe: {
     bool isCall = use.getKind() == UnsafeUse::CallToUnsafe;
     auto decl = cast_or_null<ValueDecl>(use.getDecl());
@@ -133,7 +135,8 @@ void swift::diagnoseUnsafeUse(const UnsafeUse &use) {
           [&](Type specificType) {
             ctx.Diags.diagnose(
                 loc,
-                diag::note_reference_to_unsafe_typed_decl,
+                use.getKind() == UnsafeUse::ReferenceToUnsafeStorage                       ? diag::note_unsafe_storage
+                    : diag::note_reference_to_unsafe_typed_decl,
                 isCall, decl, specificType);
           });
     } else if (type) {
@@ -376,3 +379,61 @@ void swift::diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
 
   diagnose(specificType ? specificType : type);
 }
+
+void swift::checkUnsafeStorage(NominalTypeDecl *nominal) {
+  // If the type is marked explicitly with @safe or @unsafe, there's nothing
+  // to check.
+  switch (nominal->getExplicitSafety()) {
+  case ExplicitSafety::Safe:
+  case ExplicitSafety::Unsafe:
+    return;
+
+  case ExplicitSafety::Unspecified:
+    break;
+  }
+
+  // Visitor that finds unsafe storage.
+  class UnsafeStorageVisitor: public StorageVisitor {
+    ASTContext &ctx;
+    SmallVectorImpl<UnsafeUse> &unsafeUses;
+
+  public:
+    UnsafeStorageVisitor(ASTContext &ctx, SmallVectorImpl<UnsafeUse> &unsafeUses)
+      : ctx(ctx), unsafeUses(unsafeUses) { }
+
+    bool operator()(VarDecl *property, Type propertyType) override {
+      diagnoseUnsafeType(ctx, property->getLoc(), propertyType, [&](Type type) {
+        unsafeUses.push_back(
+            UnsafeUse::forReferenceToUnsafeStorage(
+              property, propertyType, property->getLoc()));
+      });
+      return false;
+    }
+
+    bool operator()(EnumElementDecl *element, Type elementType) override {
+      diagnoseUnsafeType(ctx, element->getLoc(), elementType, [&](Type type) {
+        unsafeUses.push_back(
+            UnsafeUse::forReferenceToUnsafeStorage(
+            element, elementType, element->getLoc()));
+      });
+      return false;
+    }
+  };
+
+  // Look for any unsafe storage in this nominal type.
+  ASTContext &ctx = nominal->getASTContext();
+  SmallVector<UnsafeUse, 4> unsafeUses;
+  UnsafeStorageVisitor(ctx, unsafeUses).visit(nominal, nominal->getDeclContext());
+
+  // If we didn't find any unsafe storage, there's nothing to do.
+  if (unsafeUses.empty())
+    return;
+
+  // Complain about this type needing @safe or @unsafe.
+  nominal->diagnose(diag::decl_unsafe_storage, nominal);
+  nominal->diagnose(diag::decl_storage_mark_unsafe)
+    .fixItInsert(nominal->getAttributeInsertionLoc(false), "@unsafe ");
+  nominal->diagnose(diag::decl_storage_mark_safe)
+    .fixItInsert(nominal->getAttributeInsertionLoc(false), "@safe ");
+  std::for_each(unsafeUses.begin(), unsafeUses.end(), diagnoseUnsafeUse);
+}

lib/Sema/TypeCheckUnsafe.h

@@ -70,6 +70,9 @@ bool isUnsafeInConformance(const ValueDecl *requirement,
 void diagnoseUnsafeType(ASTContext &ctx, SourceLoc loc, Type type,
                         llvm::function_ref<void(Type)> diagnose);
 
+/// Check for unsafe storage within this nominal type declaration.
+void checkUnsafeStorage(NominalTypeDecl *nominal);
+
 }
 
 #endif // SWIFT_SEMA_TYPE_CHECK_UNSAFE_H

test/Unsafe/unsafe.swift

@@ -174,9 +174,20 @@ typealias SuperUnsafe = UnsafeSuper
 
 @unsafe typealias SuperUnsafe2 = UnsafeSuper
 
+// expected-warning@+3{{enum 'HasUnsafeThings' has storage involving unsafe types [Unsafe]}}
+// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}}{{1-1=@unsafe }}
+// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}}{{1-1=@safe }}
 enum HasUnsafeThings {
 
-case one(UnsafeSuper)
+case one(UnsafeSuper) // expected-note{{enum case 'one' involves unsafe type 'UnsafeSuper'}}
 
-@unsafe case two(UnsafeSuper)
+case two(UnsafeSuper) // expected-note{{enum case 'two' involves unsafe type 'UnsafeSuper'}}
+}
+
+// expected-warning@+3{{class 'ClassWithUnsafeStorage' has storage involving unsafe types [Unsafe]}}
+// expected-note@+2{{add '@unsafe' if this type is also unsafe to use}}{{1-1=@unsafe }}
+// expected-note@+1{{add '@safe' if this type encapsulates the unsafe storage in a safe interface}}{{1-1=@safe }}
+class ClassWithUnsafeStorage {
+  var int: Int = 0
+  var array: [UnsafeSuper]? = nil // expected-note{{property 'array' involves unsafe type 'UnsafeSuper'}}
 }