Tighten up checking for uses of unsafe conformances

Check for unsafe conformances for type erasure and opaque type
erasure.

This also uncovered an issue where we were making every conformance of
an unsafe type to an unsafe protocol @unsafe implicitly, even though
that's not really what we want.

Author: DougGregor

lib/AST/ConformanceLookupTable.cpp

@@ -53,18 +53,6 @@ DeclContext *ConformanceLookupTable::ConformanceSource::getDeclContext() const {
   llvm_unreachable("Unhandled ConformanceEntryKind in switch.");
 }
 
-bool ConformanceLookupTable::ConformanceSource::isUnsafeContext(DeclContext *dc) {
-  if (auto enclosingNominal = dc->getSelfNominalTypeDecl())
-    if (enclosingNominal->isUnsafe())
-      return true;
-
-  if (auto ext = dyn_cast<ExtensionDecl>(dc))
-    if (ext->getAttrs().hasAttribute<UnsafeAttr>())
-      return true;
-
-  return false;
-}
-
 ProtocolDecl *ConformanceLookupTable::ConformanceEntry::getProtocol() const {
   if (auto protocol = Conformance.dyn_cast<ProtocolDecl *>())
     return protocol;

lib/AST/ConformanceLookupTable.h

@@ -171,7 +171,7 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {
         options |= ProtocolConformanceFlags::Unchecked;
       if (getPreconcurrencyLoc().isValid())
         options |= ProtocolConformanceFlags::Preconcurrency;
-      if (getUnsafeLoc().isValid() || isUnsafeContext(getDeclContext()))
+      if (getUnsafeLoc().isValid())
         options |= ProtocolConformanceFlags::Unsafe;
       return options;
     }
@@ -264,10 +264,6 @@ class ConformanceLookupTable : public ASTAllocated<ConformanceLookupTable> {
     /// Get the declaration context that this conformance will be
     /// associated with.
     DeclContext *getDeclContext() const;
-
-  private:
-    /// Whether this declaration context is @unsafe.
-    static bool isUnsafeContext(DeclContext *dc);
   };
 
   /// An entry in the conformance table.

lib/Sema/TypeCheckEffects.cpp

@@ -583,6 +583,10 @@ class EffectsHandlingWalker : public ASTWalker {
       recurse = ShouldRecurse;
     } else if (auto *SVE = dyn_cast<SingleValueStmtExpr>(E)) {
       recurse = asImpl().checkSingleValueStmtExpr(SVE);
+    } else if (auto *UTO = dyn_cast<UnderlyingToOpaqueExpr>(E)) {
+      recurse = asImpl().checkWithSubstitutionMap(E, UTO->substitutions);
+    } else if (auto *EE = dyn_cast<ErasureExpr>(E)) {
+      recurse = asImpl().checkWithConformances(E, EE->getConformances());
     }
     // Error handling validation (via checkTopLevelEffects) happens after
     // type checking. If an unchecked expression is still around, the code was
@@ -1103,6 +1107,32 @@ class Classification {
     return result;
   }
 
+  /// Used when all we have is a substitution map. This can only find
+  /// "unsafe" uses.
+  static Classification forSubstitutionMap(SubstitutionMap subs,
+                                           SourceLoc loc) {
+    Classification result;
+    enumerateUnsafeUses(subs, loc, [&](UnsafeUse use) {
+      result.recordUnsafeUse(use);
+      return false;
+    });
+    return result;
+  }
+
+  /// Used when all we have is are conformances. This can only find
+  /// "unsafe" uses.
+  static Classification forConformances(
+      ArrayRef<ProtocolConformanceRef> conformances,
+      SourceLoc loc
+  ) {
+    Classification result;
+    enumerateUnsafeUses(conformances, loc, [&](UnsafeUse use) {
+      result.recordUnsafeUse(use);
+      return false;
+    });
+    return result;
+  }
+
   void merge(Classification other) {
     if (other.isInvalid())
       IsInvalid = true;
@@ -1854,6 +1884,16 @@ class ApplyClassifier {
       return ShouldRecurse;
     }
 
+    ShouldRecurse_t checkWithSubstitutionMap(Expr *E, SubstitutionMap subs) {
+      return ShouldRecurse;
+    }
+
+    ShouldRecurse_t checkWithConformances(
+        Expr *E,
+        ArrayRef<ProtocolConformanceRef> conformances) {
+      return ShouldRecurse;
+    }
+
     ConditionalEffectKind checkExhaustiveDoBody(DoCatchStmt *S) {
       // All errors thrown by the do body are caught, but any errors thrown
       // by the catch bodies are bounded by the throwing kind of the do body.
@@ -1975,6 +2015,16 @@ class ApplyClassifier {
       return ShouldRecurse;
     }
 
+    ShouldRecurse_t checkWithSubstitutionMap(Expr *E, SubstitutionMap subs) {
+      return ShouldRecurse;
+    }
+
+    ShouldRecurse_t checkWithConformances(
+        Expr *E,
+        ArrayRef<ProtocolConformanceRef> conformances) {
+      return ShouldRecurse;
+    }
+
     void visitExprPre(Expr *expr) { return; }
   };
 
@@ -3380,6 +3430,22 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     return ShouldNotRecurse;
   }
 
+  ShouldRecurse_t checkWithSubstitutionMap(Expr *E, SubstitutionMap subs) {
+    auto classification = Classification::forSubstitutionMap(
+        subs, E->getLoc());
+    checkEffectSite(E, /*requiresTry=*/false, classification);
+    return ShouldRecurse;
+  }
+
+  ShouldRecurse_t checkWithConformances(
+      Expr *E,
+      ArrayRef<ProtocolConformanceRef> conformances) {
+    auto classification = Classification::forConformances(
+        conformances, E->getLoc());
+    checkEffectSite(E, /*requiresTry=*/false, classification);
+    return ShouldRecurse;
+  }
+
   ConditionalEffectKind checkExhaustiveDoBody(DoCatchStmt *S) {
     // This is a context where errors are handled.
     ContextScope scope(*this, CurContext.withHandlesErrors());

lib/Sema/TypeCheckUnsafe.cpp

@@ -273,11 +273,10 @@ static bool forEachUnsafeConformance(
   return false;
 }
 
-bool swift::enumerateUnsafeUses(SubstitutionMap subs,
+bool swift::enumerateUnsafeUses(ArrayRef<ProtocolConformanceRef> conformances,
                                 SourceLoc loc,
                                 llvm::function_ref<bool(UnsafeUse)> fn) {
-  // FIXME: Check replacement types?
-  for (auto conformance : subs.getConformances()) {
+  for (auto conformance : conformances) {
     if (!conformance.hasEffect(EffectKind::Unsafe))
       continue;
 
@@ -295,6 +294,18 @@ bool swift::enumerateUnsafeUses(SubstitutionMap subs,
   return false;
 }
 
+bool swift::enumerateUnsafeUses(SubstitutionMap subs,
+                                SourceLoc loc,
+                                llvm::function_ref<bool(UnsafeUse)> fn) {
+  // FIXME: Check replacement types?
+
+  // Check conformances.
+  if (enumerateUnsafeUses(subs.getConformances(), loc, fn))
+    return true;
+
+  return false;
+}
+
 bool swift::isUnsafe(ConcreteDeclRef declRef) {
   return enumerateUnsafeUses(
       declRef, SourceLoc(), /*isCall=*/false, [&](UnsafeUse) {

lib/Sema/TypeCheckUnsafe.h

@@ -36,8 +36,17 @@ bool enumerateUnsafeUses(ConcreteDeclRef declRef,
                          bool isCall,
                          llvm::function_ref<bool(UnsafeUse)> fn);
 
+/// Enumerate all of the unsafe uses that occur within this array of protocol
+/// conformances.
+///
+/// The given `fn` will be called with each unsafe use. If it returns `true`
+/// for any use, this function will return `true` immediately. Otherwise,
+/// it will return `false` once all unsafe uses have been emitted.
+bool enumerateUnsafeUses(ArrayRef<ProtocolConformanceRef> conformances,
+                         SourceLoc loc,
+                         llvm::function_ref<bool(UnsafeUse)> fn);
+
 /// Enumerate all of the unsafe uses that occur within this substitution map.
-/// reference.
 ///
 /// The given `fn` will be called with each unsafe use. If it returns `true`
 /// for any use, this function will return `true` immediately. Otherwise,

test/Unsafe/safe.swift

@@ -71,6 +71,15 @@ func testConformance(i: Int) {
   acceptP(i) // expected-note{{@unsafe conformance of 'Int' to protocol 'P' involves unsafe code}}
 }
 
+func returnsOpaqueP() -> some P {
+  5 // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  // expected-note@-1{{@unsafe conformance of 'Int' to protocol 'P' involves unsafe code}}
+}
+
+func returnsExistentialP() -> any P {
+  5 // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}
+  // expected-note@-1{{@unsafe conformance of 'Int' to protocol 'P' involves unsafe code}}
+}
 
 // Parsing of `unsafe` expressions.
 func testUnsafePositionError() -> Int {

test/Unsafe/unsafe-suppression.swift

@@ -60,12 +60,21 @@ extension S3: P {
 
 struct S4 { }
 
-@unsafe
-extension S4: P {
+extension S4: @unsafe P {
   @unsafe
   func protoMethod() { } // okay
 }
 
+protocol P2 {
+  func proto2Method()
+}
+
+@unsafe
+extension S4: P2 { // expected-warning{{conformance of 'S4' to protocol 'P2' involves unsafe code; use '@unsafe' to indicate that the conformance is not memory-safe}}
+  @unsafe
+  func proto2Method() { } // expected-note{{unsafe instance method}}
+}
+
 
 @unsafe
 class SendableC1: @unchecked Sendable { }