Merge pull request #41545 from slavapestov/non-final-class-self-same-type-requirement-conformance

Sema: Warn about non-final classes conforming to protocols with a same-type requirement on 'Self'

Author: slavapestov

CHANGELOG.md

@@ -5,6 +5,48 @@ _**Note:** This is in reverse chronological order, so newer entries are added to
 
 ## Swift 5.7
 
+* The compiler now emits a warning when a non-final class conforms to a protocol that imposes a same-type requirement between `Self` and an associated type. This is because such a requirement makes the conformance unsound for subclasses.
+
+For example, Swift 5.6 would allow the following code, which at runtime would construct an instanec of `C` and not `SubC` as expected:
+
+  ```swift
+  protocol P {
+    associatedtype A : Q where Self == Self.A.B
+  }
+  
+  protocol Q {
+    associatedtype B
+  
+    static func getB() -> B
+  }
+  
+  class C : P {
+    typealias A = D
+  }
+  
+  class D : Q {
+    typealias B = C
+  
+    static func getB() -> C { return C() }
+  }
+  
+  extension P {
+    static func getAB() -> Self {
+      // This is well-typed, because `Self.A.getB()` returns
+      // `Self.A.B`, which is equivalent to `Self`.
+      return Self.A.getB()
+    }
+  }
+  
+  class SubC : C {}
+  
+  // P.getAB() declares a return type of `Self`, so it should
+  // return `SubC`, but it actually returns a `C`.
+  print(SubC.getAB())
+  ```
+
+  To make the above example correct, either the class `C` needs to become `final` (in which case `SubC` cannot be declared) or protocol `P` needs to be re-designed to not include the same-type requirement `Self == Self.A.B`.
+
 * [SE-0341][]:
 
   Opaque types can now be used in the parameters of functions and subscripts, wher they provide a shorthand syntax for the introduction of a generic parameter. For example, the following:

include/swift/AST/DiagnosticsSema.def

@@ -1944,6 +1944,11 @@ ERROR(type_does_not_conform,none,
 WARNING(type_does_not_conform_swiftui_warning,none,
         "type %0 does not conform to protocol %1", (Type, Type))
 
+ERROR(non_final_class_cannot_conform_to_self_same_type,none,
+      "non-final class %0 cannot safely conform to protocol %1, "
+      "which requires that 'Self' is exactly equal to %2",
+      (Type, Type, Type))
+
 ERROR(cannot_use_nil_with_this_type,none,
       "'nil' cannot be used in context expecting type %0", (Type))
 

lib/Sema/TypeCheckProtocol.cpp

@@ -4853,18 +4853,42 @@ void ConformanceChecker::ensureRequirementsAreSatisfied() {
 
   CheckedRequirementSignature = true;
 
+  auto &diags = proto->getASTContext().Diags;
+
   auto DC = Conformance->getDeclContext();
   auto substitutingType = DC->mapTypeIntoContext(Conformance->getType());
   auto substitutions = SubstitutionMap::getProtocolSubstitutions(
       proto, substitutingType, ProtocolConformanceRef(Conformance));
 
+  auto reqSig = proto->getRequirementSignature().getRequirements();
+
   auto result = TypeChecker::checkGenericArguments(
       DC->getParentModule(), Loc, Loc,
       // FIXME: maybe this should be the conformance's type
       proto->getDeclaredInterfaceType(),
       { proto->getSelfInterfaceType() },
-      proto->getRequirementSignature().getRequirements(),
-      QuerySubstitutionMap{substitutions});
+      reqSig, QuerySubstitutionMap{substitutions});
+
+  // Non-final classes should not be able to conform to protocols with a
+  // same-type requirement on 'Self', since such a conformance would no
+  // longer be covariant. For now, this is a warning. Once this becomes
+  // an error, we can handle it as part of the above checkGenericArguments()
+  // call by passing in a superclass-bound archetype for the 'self' type
+  // instead of the concrete class type itself.
+  if (auto *classDecl = Adoptee->getClassOrBoundGenericClass()) {
+    if (!classDecl->isSemanticallyFinal()) {
+      for (auto req : reqSig) {
+        if (req.getKind() == RequirementKind::SameType &&
+            req.getFirstType()->isEqual(proto->getSelfInterfaceType())) {
+          diags.diagnose(Loc, diag::non_final_class_cannot_conform_to_self_same_type,
+                         Adoptee, proto->getDeclaredInterfaceType(),
+                         req.getSecondType())
+              .warnUntilSwiftVersion(6);
+          break;
+        }
+      }
+    }
+  }
 
   switch (result) {
   case RequirementCheckResult::Success:
@@ -4879,9 +4903,9 @@ void ConformanceChecker::ensureRequirementsAreSatisfied() {
     // Diagnose the failure generically.
     // FIXME: Would be nice to give some more context here!
     if (!Conformance->isInvalid()) {
-      proto->getASTContext().Diags.diagnose(Loc, diag::type_does_not_conform,
-                                            Adoptee,
-                                            Proto->getDeclaredInterfaceType());
+      diags.diagnose(Loc, diag::type_does_not_conform,
+                     Adoptee,
+                     Proto->getDeclaredInterfaceType());
       Conformance->setInvalid();
     }
     return;

test/Generics/sr13884.swift renamed to test/Generics/non_final_class_conforms_same_type_requirement_on_self.swift

@@ -9,7 +9,11 @@ public protocol Q {
   associatedtype B
 }
 
+// This is rejected, because 'A.B == Self' means that 'Self' must
+// exactly equal C; since C is not final, this means the conformance
+// is not covariant.
 public class C : P {
+// expected-warning@-1 {{non-final class 'C' cannot safely conform to protocol 'P', which requires that 'Self' is exactly equal to 'Self.A.B'; this is an error in Swift 6}}
   public typealias A = D
 }
 

test/Generics/rdar80503090.swift

@@ -19,6 +19,7 @@ extension P where T : Q {
 }
 
 class C : P {}
+// expected-warning@-1 {{non-final class 'C' cannot safely conform to protocol 'P', which requires that 'Self' is exactly equal to 'Self.T'; this is an error in Swift 6}}
 
 extension P where T : C {
   // CHECK-LABEL: Generic signature: <Self where Self == C>

validation-test/compiler_crashers_2_fixed/0189-sr10033.swift

@@ -14,7 +14,9 @@ extension P2 {
 }
 
 class C1 : P1 {
+// expected-warning@-1 {{non-final class 'C1' cannot safely conform to protocol 'P1', which requires that 'Self' is exactly equal to 'Self.A2.A1'; this is an error in Swift 6}}
   class A2 : P2 {
+  // expected-warning@-1 {{non-final class 'C1.A2' cannot safely conform to protocol 'P2', which requires that 'Self' is exactly equal to 'Self.A1.A2'; this is an error in Swift 6}}
     typealias A1 = C1
   }
 }