Account for implicit unsafety in unsafe witness and override checks

When a witness is unsafe and its corresponding requirement is not
unsafe, the conformance itself needs to be determined to be unsafe.
When doing this check, account for the fact that the requirement might
be implicitly unsafe, i.e., it uses unsafe types. In such cases, the
requirement is effectively unsafe, so the unsafe witness to it does not
make the conformance unsafe. Therefore, suppress the diagnostic. This
accounts for cases where the protocol comes from a module that didn't
enable strict memory safety checking, or didn't suppress all warnings
related to it, as well as cases where substitution of type witnesses
introduces the unsafe type.

The same thing occurs with overriding a method: an unsafe override of
a method that involves unsafe types (but was not marked @unsafe for
whatever reason) does not make the subclassing itself unsafe, so don't
diagnose it as such.

Author: DougGregor

lib/Sema/TypeCheckAvailability.h

@@ -280,6 +280,10 @@ void diagnoseUnsafeUse(const UnsafeUse &use, bool asNote = false);
 /// declaration, if there are any.
 void diagnoseUnsafeUsesIn(const Decl *decl);
 
+/// Determine whether a reference to this declaration is considered unsafe,
+/// either explicitly (@unsafe) or because it references an unsafe type.
+bool isUnsafe(ConcreteDeclRef declRef);
+
 } // namespace swift
 
 #endif // SWIFT_SEMA_TYPE_CHECK_AVAILABILITY_H

lib/Sema/TypeCheckDeclOverride.cpp

@@ -2249,14 +2249,20 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
     diagnoseOverrideForAvailability(override, base);
   }
 
-  if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe) &&
-      override->isUnsafe() && !base->isUnsafe()) {
-    // Don't diagnose @unsafe overrides if the subclass is @unsafe.
-    auto overridingClass = override->getDeclContext()->getSelfClassDecl();
-    bool shouldDiagnose = !overridingClass || !overridingClass->allowsUnsafe();
-
-    if (shouldDiagnose) {
-      diagnoseUnsafeUse(UnsafeUse::forOverride(override, base));
+  if (ctx.LangOpts.hasFeature(Feature::WarnUnsafe)) {
+    // If the override is unsafe but the base declaration is not, then the
+    // inheritance itself is unsafe.
+    auto subs = SubstitutionMap::getOverrideSubstitutions(base, override);
+    ConcreteDeclRef overrideRef(override);
+    ConcreteDeclRef baseRef(base, subs);
+    if (isUnsafe(overrideRef) && !isUnsafe(baseRef)) {
+      // Don't diagnose @unsafe overrides if the subclass is @unsafe.
+      auto overridingClass = override->getDeclContext()->getSelfClassDecl();
+      bool shouldDiagnose = !overridingClass || !overridingClass->allowsUnsafe();
+
+      if (shouldDiagnose) {
+        diagnoseUnsafeUse(UnsafeUse::forOverride(override, base));
+      }
     }
   }
   /// Check attributes associated with the base; some may need to merged with

lib/Sema/TypeCheckProtocol.cpp

@@ -2577,8 +2577,14 @@ checkIndividualConformance(NormalProtocolConformance *conformance) {
       if (!valueRequirement)
         continue;
 
+      // If the witness is unsafe and the requirement is not effectively
+      // unsafe, then the conformance must be unsafe.
       if (auto witness = conformance->getWitnessUncached(valueRequirement)) {
-        if (witness.getDecl()->isUnsafe() && !requirement->isUnsafe()) {
+        if (isUnsafe(witness.getDeclRef()) &&
+            !isUnsafe(
+                ConcreteDeclRef(
+                  valueRequirement,
+                  witness.getRequirementToWitnessThunkSubs()))) {
           unsafeUses.push_back(
               UnsafeUse::forWitness(
                 witness.getDecl(), requirement, conformance));

lib/Sema/TypeCheckUnsafe.cpp

@@ -280,3 +280,21 @@ void swift::diagnoseUnsafeUsesIn(const Decl *decl) {
     diagnoseUnsafeUse(unsafeUse, /*asNote=*/true);
   }
 }
+
+bool swift::isUnsafe(ConcreteDeclRef declRef) {
+  auto decl = declRef.getDecl();
+  if (!decl)
+    return false;
+
+  // Is the declaration explicitly @unsafe?
+  if (decl->isUnsafe())
+    return true;
+
+  auto type = decl->getInterfaceType();
+  if (auto subs = declRef.getSubstitutions())
+    type = type.subst(subs);
+  if (type->isUnsafe())
+    return true;
+
+  return false;
+}

test/Unsafe/Inputs/unsafe_swift_decls.swift

@@ -13,3 +13,11 @@ public protocol Ptrable {
 }
 
 extension HasAPointerType: Ptrable { }
+
+public protocol HasUnsafeRequirement {
+  func f(_: PointerType)
+}
+
+open class SuperclassWithUnsafeMethod {
+  open func implicitlyUnsafe(_: PointerType) { }
+}

test/Unsafe/unsafe.swift

@@ -50,7 +50,7 @@ struct ConformsToMultiP { }
 // expected-warning@+1{{conformance of 'ConformsToMultiP' to protocol 'MultiP' involves unsafe code; use '@unsafe' to indicate that the conformance is not memory-safe [Unsafe]}}{{29-29=@unsafe }}
 extension ConformsToMultiP: MultiP {
   // expected-note@-1{{unsafe type 'UnsafeSuper' cannot satisfy safe associated type 'Ptr'}}
-  @unsafe func f() -> UnsafeSuper { .init() } // expected-note{{unsafe instance method 'f()' cannot satisfy safe requirement}}
+  @unsafe func f() -> UnsafeSuper { .init() }
 }
 
 // -----------------------------------------------------------------------

test/Unsafe/unsafe_imports.swift

@@ -24,3 +24,12 @@ func testUnsafe(_ ut: UnsafeType) { // expected-note{{reference to unsafe struct
 // expected-warning@+1{{global function 'testUnsafeThroughAlias' involves unsafe code; use '@unsafe' to indicate that its use is not memory-safe}}
 func testUnsafeThroughAlias(_ ut: UnsafeTypeAlias) { // expected-note{{reference to type alias 'UnsafeTypeAlias' whose underlying type involves unsafe type 'PointerType'}}
 }
+
+
+struct ConformsToUnsafeRequirement: HasUnsafeRequirement {
+  @unsafe func f(_: PointerType) { }
+}
+
+class SubclassWithUnsafeMethod: SuperclassWithUnsafeMethod {
+  @unsafe override func implicitlyUnsafe(_: PointerType) { }
+}