Merge pull request #9830 from DougGregor/same-type-self-soundness-hole

Author: swift-ci

include/swift/AST/Decl.h

@@ -3418,46 +3418,54 @@ class ClassDecl : public NominalTypeDecl {
 struct SelfReferenceKind {
   bool result;
   bool parameter;
+  bool requirement;
   bool other;
 
   /// The type does not refer to 'Self' at all.
   static SelfReferenceKind None() {
-    return SelfReferenceKind(false, false, false);
+    return SelfReferenceKind(false, false, false, false);
   }
 
   /// The type refers to 'Self', but only as the result type of a method.
   static SelfReferenceKind Result() {
-    return SelfReferenceKind(true, false, false);
+    return SelfReferenceKind(true, false, false, false);
   }
 
   /// The type refers to 'Self', but only as the parameter type of a method.
   static SelfReferenceKind Parameter() {
-    return SelfReferenceKind(false, true, false);
+    return SelfReferenceKind(false, true, false, false);
+  }
+
+  /// The type refers to 'Self' within a same-type requiement.
+  static SelfReferenceKind Requirement() {
+    return SelfReferenceKind(false, false, true, false);
   }
 
   /// The type refers to 'Self' in a position that is invariant.
   static SelfReferenceKind Other() {
-    return SelfReferenceKind(false, false, true);
+    return SelfReferenceKind(false, false, false, true);
   }
 
   SelfReferenceKind flip() const {
-    return SelfReferenceKind(parameter, result, other);
+    return SelfReferenceKind(parameter, result, requirement, other);
   }
 
   SelfReferenceKind operator|=(SelfReferenceKind kind) {
     result |= kind.result;
+    requirement |= kind.requirement;
     parameter |= kind.parameter;
     other |= kind.other;
     return *this;
   }
 
   operator bool() const {
-    return result || parameter || other;
+    return result || parameter || requirement || other;
   }
 
 private:
-  SelfReferenceKind(bool result, bool parameter, bool other)
-    : result(result), parameter(parameter), other(other) { }
+  SelfReferenceKind(bool result, bool parameter, bool requirement, bool other)
+    : result(result), parameter(parameter), requirement(requirement),
+      other(other) { }
 };
 
 /// ProtocolDecl - A declaration of a protocol, for example:

include/swift/AST/DiagnosticsSema.def

@@ -1413,6 +1413,19 @@ ERROR(witness_self_non_subtype,none,
       "(%2) because it uses 'Self' in a non-parameter, non-result type "
       "position",
       (Type, DeclName, Type))
+ERROR(witness_self_same_type,none,
+      "%0 %1 in non-final class %2 cannot be used to satisfy requirement %3 %4"
+      " (in protocol %5) due to same-type requirement involving 'Self'",
+      (DescriptiveDeclKind, DeclName, Type, DescriptiveDeclKind,
+       DeclName, Type))
+WARNING(witness_self_same_type_warn,none,
+      "%0 %1 in non-final class %2 cannot be used to satisfy requirement %3 %4"
+      " (in protocol %5) due to same-type requirement involving 'Self'",
+      (DescriptiveDeclKind, DeclName, Type, DescriptiveDeclKind,
+       DeclName, Type))
+NOTE(witness_self_weaken_same_type,none,
+      "consider weakening the same-type requirement %0 == %1 to a superclass "
+      "requirement", (Type, Type))
 ERROR(witness_requires_dynamic_self,none,
       "method %0 in non-final class %1 must return `Self` to conform to "
       "protocol %2",

lib/AST/Decl.cpp

@@ -3043,6 +3043,25 @@ findProtocolSelfReferences(const ProtocolDecl *proto, Type type,
   return SelfReferenceKind::None();
 }
 
+/// Find Self references in a generic signature's same-type requirements.
+static SelfReferenceKind
+findProtocolSelfReferences(const ProtocolDecl *protocol,
+                           GenericSignature *genericSig){
+  if (!genericSig) return SelfReferenceKind::None();
+
+  auto selfTy = protocol->getSelfInterfaceType();
+  for (const auto &req : genericSig->getRequirements()) {
+    if (req.getKind() != RequirementKind::SameType)
+      continue;
+
+    if (req.getFirstType()->isEqual(selfTy) ||
+        req.getSecondType()->isEqual(selfTy))
+      return SelfReferenceKind::Requirement();
+  }
+
+  return SelfReferenceKind::None();
+}
+
 /// Find Self references within the given requirement.
 SelfReferenceKind
 ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
@@ -3062,7 +3081,7 @@ ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
   if (type->hasError())
     return SelfReferenceKind::None();
 
-  if (isa<AbstractFunctionDecl>(value)) {
+  if (auto func = dyn_cast<AbstractFunctionDecl>(value)) {
     // Skip the 'self' parameter.
     type = type->castTo<AnyFunctionType>()->getResult();
 
@@ -3076,9 +3095,24 @@ ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
         return SelfReferenceKind::Other();
     }
 
+    // Check the requirements of a generic function.
+    if (func->isGeneric()) {
+      if (auto result =
+            ::findProtocolSelfReferences(this, func->getGenericSignature()))
+        return result;
+    }
+
     return ::findProtocolSelfReferences(this, type,
                                         skipAssocTypes);
-  } else if (isa<SubscriptDecl>(value)) {
+  } else if (auto subscript = dyn_cast<SubscriptDecl>(value)) {
+    // Check the requirements of a generic subscript.
+    if (subscript->isGeneric()) {
+      if (auto result =
+            ::findProtocolSelfReferences(this,
+                                         subscript->getGenericSignature()))
+        return result;
+    }
+
     return ::findProtocolSelfReferences(this, type,
                                         skipAssocTypes);
   } else {

lib/Sema/TypeCheckProtocol.cpp

@@ -2794,6 +2794,50 @@ witnessHasImplementsAttrForRequirement(ValueDecl *witness,
   return false;
 }
 
+/// Determine the given witness has a same-type constraint constraining the
+/// given 'Self' type, and return the
+///
+/// \returns None if there is no such constraint; a non-empty optional that
+/// may have the \c RequirementRepr for the actual constraint.
+static Optional<RequirementRepr *>
+getAdopteeSelfSameTypeConstraint(ClassDecl *selfClass, ValueDecl *witness) {
+  auto genericSig =
+    witness->getInnermostDeclContext()->getGenericSignatureOfContext();
+  if (!genericSig) return None;
+
+  for (const auto &req : genericSig->getRequirements()) {
+    if (req.getKind() != RequirementKind::SameType)
+      continue;
+
+    if (req.getFirstType()->getAnyNominal() == selfClass ||
+        req.getSecondType()->getAnyNominal() == selfClass) {
+      // Try to find the requirement-as-written.
+      GenericParamList *genericParams = nullptr;
+
+      if (auto func = dyn_cast<AbstractFunctionDecl>(witness))
+        genericParams = func->getGenericParams();
+      else if (auto subscript = dyn_cast<SubscriptDecl>(witness))
+        genericParams = subscript->getGenericParams();
+      if (genericParams) {
+        for (auto &req : genericParams->getRequirements()) {
+          if (req.getKind() != RequirementReprKind::SameType)
+            continue;
+
+          if (req.getFirstType()->getAnyNominal() == selfClass ||
+              req.getSecondType()->getAnyNominal() == selfClass)
+            return &req;
+        }
+      }
+
+      // Form an optional(nullptr) to indicate that we don't have the
+      // requirement itself.
+      return nullptr;
+    }
+  }
+
+  return None;
+}
+
 ResolveWitnessResult
 ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
   assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
@@ -3076,6 +3120,31 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
                              conformance->getType());
             });
         }
+      } else if (selfKind.requirement) {
+        if (auto constraint = getAdopteeSelfSameTypeConstraint(classDecl,
+                                                               witness)) {
+          // A "Self ==" constraint works incorrectly with subclasses. Complain.
+          auto proto = Conformance->getProtocol();
+          auto &diags = proto->getASTContext().Diags;
+          diags.diagnose(witness->getLoc(),
+                         proto->getASTContext().LangOpts.isSwiftVersion3()
+                           ? diag::witness_self_same_type_warn
+                           : diag::witness_self_same_type,
+                         witness->getDescriptiveKind(),
+                         witness->getFullName(),
+                         Conformance->getType(),
+                         requirement->getDescriptiveKind(),
+                         requirement->getFullName(),
+                         proto->getDeclaredType());
+
+          if (auto requirementRepr = *constraint) {
+            diags.diagnose(requirementRepr->getEqualLoc(),
+                           diag::witness_self_weaken_same_type,
+                           requirementRepr->getFirstType(),
+                           requirementRepr->getSecondType())
+              .fixItReplace(requirementRepr->getEqualLoc(), ":");
+          }
+        }
       }
 
       // A non-final class can model a protocol requirement with a

test/Compatibility/self_same_type.swift

@@ -0,0 +1,22 @@
+// RUN: %target-typecheck-verify-swift -swift-version 3
+
+protocol P {
+  associatedtype T
+}
+
+protocol Q {
+  func foo<T: P>(_: T, _: T.T) where T.T == Self
+}
+
+class C1: Q {
+  func foo<T: P>(_: T, _: C1) where T.T == C1 {} // expected-warning{{instance method 'foo' in non-final class 'C1' cannot be used to satisfy requirement instance method 'foo' (in protocol 'Q') due to same-type requirement involving 'Self'}}}}
+    // expected-note@-1{{consider weakening the same-type requirement 'T.T' == 'C1' to a superclass requirement}}{{41-43=:}}
+}
+
+final class C2: Q {
+  func foo<T: P>(_: T, _: C2) where T.T == C2 {}
+}
+
+class C3: Q {
+  func foo<T: P>(_: T, _: C3) where T.T: C3 {}
+}

test/decl/protocol/conforms/self_same_type.swift

@@ -0,0 +1,22 @@
+// RUN: %target-typecheck-verify-swift -swift-version 4
+
+protocol P {
+  associatedtype T
+}
+
+protocol Q {
+  func foo<T: P>(_: T, _: T.T) where T.T == Self
+}
+
+class C1: Q {
+  func foo<T: P>(_: T, _: C1) where T.T == C1 {} // expected-error{{instance method 'foo' in non-final class 'C1' cannot be used to satisfy requirement instance method 'foo' (in protocol 'Q') due to same-type requirement involving 'Self'}}}}
+    // expected-note@-1{{consider weakening the same-type requirement 'T.T' == 'C1' to a superclass requirement}}{{41-43=:}}
+}
+
+final class C2: Q {
+  func foo<T: P>(_: T, _: C2) where T.T == C2 {}
+}
+
+class C3: Q {
+  func foo<T: P>(_: T, _: C3) where T.T: C3 {}
+}