Only permit inheritance of protocol conformance when it is semantically valid.

A protocol conformance of a class A to a protocol P can be inherited
by a subclass B of A unless
  - A requirement of P refers to Self (not an associated type thereof)
  in its signature, 
    + *except* when Self is the result type of the method in P and the
    corresponding witness for A's conformance to B is a DynamicSelf
    method.

Remove the uses of DynamicSelf from the literal protocols, going back
to Self. The fact that the conformances of NSDictionary, NSArray,
NSString, etc. to the corresponding literal protocols use witnesses
that return DynamicSelf makes NSMutableDictionary, NSMutableArray,
NSMutableString, and other subclasses still conform to the
protocol. We also correctly reject attempts to (for example) create an
NSDecimalNumber from a numeric literal, because NSNumber doesn't
provide a suitable factory method by which any subclass can be literal
convertible.



Swift SVN r14204

Author: DougGregor

include/swift/AST/ProtocolConformance.h

@@ -29,6 +29,7 @@
 namespace swift {
 
 class ASTContext;
+class DiagnosticEngine;
 class GenericParamList;
 class NormalProtocolConformance;
 class ProtocolConformance;
@@ -199,7 +200,10 @@ class ProtocolConformance {
 
   /// Get the underlying normal conformance.
   const NormalProtocolConformance *getRootNormalConformance() const;
-  
+
+  /// Determine whether this conformance is inheritable by subclasses.
+  bool isInheritable(LazyResolver *resolver) const;
+
   /// Determine whether the witness for the given requirement
   /// is either the default definition or was otherwise deduced.
   ///
@@ -253,16 +257,28 @@ class ProtocolConformance {
 /// providing the witnesses \c A.foo and \c B<T>.foo, respectively, for the
 /// requirement \c foo.
 class NormalProtocolConformance : public ProtocolConformance {
+  /// Describes whether this conformance is inheritable to subclasses or not.
+  enum class IsInheritableKind {
+    /// We haven't checked yet whether this conformance is inheritable.
+    Unknown,
+    /// This conformance is inheritable.
+    Inheritable,
+    /// Ths conformance is not inheritable.
+    NotInheritable
+  };
+
   /// \brief The protocol being conformed to and its current state.
   llvm::PointerIntPair<ProtocolDecl *, 2, ProtocolConformanceState>
     ProtocolAndState;
 
   /// The location of this protocol conformance in the source.
   SourceLoc Loc;
 
-  /// \brief The declaration context containing the ExtensionDecl or
-  /// NominalTypeDecl that declared the conformance.
-  DeclContext *DC;
+  /// The declaration context containing the ExtensionDecl or
+  /// NominalTypeDecl that declared the conformance, along with whether this
+  /// conformance is inheritable.
+  mutable llvm::PointerIntPair<DeclContext *, 2, IsInheritableKind> 
+    DCAndInheritable;
 
   /// \brief The mapping of individual requirements in the protocol over to
   /// the declarations that satisfy those requirements.
@@ -286,10 +302,13 @@ class NormalProtocolConformance : public ProtocolConformance {
                             SourceLoc loc, DeclContext *dc,
                             ProtocolConformanceState state)
     : ProtocolConformance(ProtocolConformanceKind::Normal, conformingType),
-      ProtocolAndState(protocol, state), Loc(loc), DC(dc)
+      ProtocolAndState(protocol, state), Loc(loc),
+      DCAndInheritable(dc, IsInheritableKind::Unknown)
   {
   }
 
+  bool isInheritableSlow(LazyResolver *resolver) const;
+
 public:
   /// Get the protocol being conformed to.
   ProtocolDecl *getProtocol() const { return ProtocolAndState.getPointer(); }
@@ -300,13 +319,13 @@ class NormalProtocolConformance : public ProtocolConformance {
   /// Get the declaration context that contains the conforming extension or
   /// nominal type declaration.
   DeclContext *getDeclContext() const {
-    return DC;
+    return DCAndInheritable.getPointer();
   }
 
   /// Set the declaration context that contains the conforming extension or
   /// nominal type declaration.
   void setDeclContext(DeclContext *dc) {
-    DC = dc;
+    DCAndInheritable.setPointer(dc);
   }
 
   /// Retrieve the state of this conformance.
@@ -319,6 +338,20 @@ class NormalProtocolConformance : public ProtocolConformance {
     ProtocolAndState.setInt(state);
   }
 
+  /// Determine whether this conformance is inheritable by subclasses.
+  bool isInheritable(LazyResolver *resolver) const {
+    switch (DCAndInheritable.getInt()) {
+    case IsInheritableKind::Inheritable:
+      return true;
+
+    case IsInheritableKind::NotInheritable:
+      return false;
+
+    case IsInheritableKind::Unknown:
+      return isInheritableSlow(resolver);
+    }
+  }
+
   /// Retrieve the type witness corresponding to the given associated type
   /// requirement.
   const Substitution &getTypeWitness(AssociatedTypeDecl *assocType, 
@@ -384,6 +417,7 @@ class NormalProtocolConformance : public ProtocolConformance {
     DefaultedDefinitions.insert(requirement);
   }
 
+
   static bool classof(const ProtocolConformance *conformance) {
     return conformance->getKind() == ProtocolConformanceKind::Normal;
   }
@@ -459,6 +493,11 @@ class SpecializedProtocolConformance : public ProtocolConformance,
     return GenericConformance->getState();
   }
 
+  /// Determine whether this conformance is inheritable by subclasses.
+  bool isInheritable(LazyResolver *resolver) const {
+    return GenericConformance->isInheritable(resolver);
+  }
+
   /// Retrieve the type witness for the given associated type.
   const Substitution &getTypeWitness(AssociatedTypeDecl *assocType, 
                                      LazyResolver *resolver) const;
@@ -547,6 +586,12 @@ class InheritedProtocolConformance : public ProtocolConformance,
     return InheritedConformance->getState();
   }
 
+  /// Determine whether this conformance is inheritable by subclasses.
+  bool isInheritable(LazyResolver *resolver) const {
+    // Always inheritable, because it was itself inherited.
+    return true;
+  }
+
   /// Retrieve the type witness for the given associated type.
   const Substitution &getTypeWitness(AssociatedTypeDecl *assocType, 
                                      LazyResolver *resolver) const {

lib/AST/Module.cpp

@@ -697,6 +697,12 @@ LookupConformanceResult Module::lookupConformance(Type type,
       return inheritedConformance;
 
     case ConformanceKind::Conforms:
+      // If the inherited conformance is not in fact inheritable, then
+      // we're done.
+      if (!inheritedConformance.getPointer()->isInheritable(resolver)) {
+        return { nullptr, ConformanceKind::DoesNotConform };
+      }
+
       // Create inherited conformance below.
       break;
     }

lib/AST/ProtocolConformance.cpp

@@ -20,6 +20,7 @@
 #include "swift/AST/Module.h"
 #include "swift/AST/Substitution.h"
 #include "swift/AST/Types.h"
+#include "swift/AST/TypeWalker.h"
 
 using namespace swift;
 
@@ -270,6 +271,10 @@ ProtocolConformance::getRootNormalConformance() const {
   return cast<NormalProtocolConformance>(C);
 }
 
+bool ProtocolConformance::isInheritable(LazyResolver *resolver) const {
+  CONFORMANCE_SUBCLASS_DISPATCH(isInheritable, (resolver))
+}
+
 ProtocolConformance *ProtocolConformance::subst(Module *module,
                                                 Type substType,
                                                 ArrayRef<Substitution> subs,
@@ -369,3 +374,127 @@ ProtocolConformance::getInheritedConformance(ProtocolDecl *protocol) const {
          && "inherited conformance does not match type");
   return foundInherited;
 }
+
+namespace {
+  /// Describes whether a requirement refers to 'Self', for use in the
+  /// is-inheritable check.
+  enum class SelfReferenceKind {
+    /// The type does not refer to 'Self' at all.
+    No,
+    /// The type refers to 'Self', but only as the result type of a method.
+    Result,
+    /// The type refers to 'Self' in some position that is not the result type
+    /// of a method.
+    Yes
+  };
+}
+
+/// Determine whether the given type is the 'Self' generic parameter
+/// of a protocol.
+static bool isSelf(Type type) {
+  if (auto genericParam = type->getAs<GenericTypeParamType>()) {
+    return genericParam->getDepth() == 0 && genericParam->getIndex() == 0;
+  }
+
+  return false;
+}
+
+/// Determine whether the given type contains a reference to the
+/// 'Self' generic parameter of a protocol that is not the base of a
+/// dependent member expression.
+static bool containsSelf(Type type) {
+  struct SelfWalker : public TypeWalker {
+    bool FoundSelf = false;
+
+    virtual Action walkToTypePre(Type ty) { 
+      // If we found a reference to 'Self', note it and stop.
+      if (isSelf(ty)) {
+        FoundSelf = true;
+        return Action::Stop;
+      }
+
+      // Don't recurse into the base of a dependent member type: it
+      // doesn't contain a bare 'Self'.
+      if (ty->is<DependentMemberType>())
+        return Action::SkipChildren;
+
+      return Action::Continue; 
+    }
+  } selfWalker;
+
+  type.walk(selfWalker);
+  return selfWalker.FoundSelf;
+}
+
+/// Find the bare Self references within the given requirement.
+static SelfReferenceKind findSelfReferences(ValueDecl *value) {
+  // Types never refer to 'Self'.
+  if (isa<TypeDecl>(value))
+    return SelfReferenceKind::No;
+
+  // If the function requirement returns Self and has no other
+  // reference to Self, note that.
+  if (auto func = dyn_cast<FuncDecl>(value)) {
+    auto type = func->getInterfaceType();
+    // Skip the 'self' type.
+    type = type->castTo<AnyFunctionType>()->getResult();
+    for (unsigned i = 1, n = func->getNumParamPatterns(); i != n; ++i) {
+      // Check whether the input type contains Self anywhere.
+      auto fnType = type->castTo<AnyFunctionType>();
+      if (containsSelf(fnType->getInput()))
+        return SelfReferenceKind::Yes;
+
+      type = fnType->getResult();
+    }
+
+    // The result type remains.
+    return (isSelf(type) || func->hasDynamicSelf())
+             ? SelfReferenceKind::Result
+             : containsSelf(type) ? SelfReferenceKind::Yes
+                                  : SelfReferenceKind::No;
+  }
+
+  return containsSelf(value->getInterfaceType()) ? SelfReferenceKind::Yes
+                                                 : SelfReferenceKind::No;
+}
+
+bool NormalProtocolConformance::isInheritableSlow(LazyResolver *resolver) const{
+  auto classDecl = getType()->getClassOrBoundGenericClass();
+  assert(classDecl && "Conformance can't be inheritable, ever");
+  
+  for (auto member : getProtocol()->getMembers()) {
+    auto req = dyn_cast<ValueDecl>(member);
+    if (!req)
+      continue;
+    
+    // Skip accessors.
+    if (isa<FuncDecl>(req) && cast<FuncDecl>(req)->isAccessor())
+      continue;
+      
+    // Check the kinds of references to Self that show up in the given
+    // requirement.
+    switch (findSelfReferences(req)) {
+    case SelfReferenceKind::No:
+      continue;
+
+    case SelfReferenceKind::Result: {
+      // When only the result type is 'Self', we can inherit the
+      // conformance if the witness returns DynamicSelf.
+      auto func = cast_or_null<FuncDecl>(
+                    getWitness(req, resolver).getDecl());
+      if (func && func->hasDynamicSelf())
+        continue;
+      // Fall through
+    }
+
+    case SelfReferenceKind::Yes: {
+      DCAndInheritable.setInt(IsInheritableKind::NotInheritable);
+      return false;
+    }
+    }
+  }
+
+  // 
+  DCAndInheritable.setInt(IsInheritableKind::Inheritable);
+  return true;
+}

test/decl/protocol/conforms/inherited.swift

@@ -0,0 +1,95 @@
+// RUN: %swift -parse %s -verify
+
+// Never inheritable: method with 'Self' in its signature
+protocol P1 {
+  func f1(x: Self) -> Bool
+}
+
+// Never inheritable: property with 'Self' in its signature.
+protocol P2 {
+  var prop2: Self { get }
+}
+
+// Never inheritable: subscript with 'Self' in its signature
+protocol P3 {
+  subscript (i: Int) -> Self { get }
+}
+
+// Never inheritable: subscript with 'Self' in its signature
+protocol P4 {
+  subscript (s: Self) -> Int { get }
+}
+
+// Potentially inheritable: method returning Self
+protocol P5 {
+  func f5() -> Self
+}
+
+// Inheritable: method returning DynamicSelf
+protocol P6 {
+  func f6() -> DynamicSelf
+}
+
+// Inheritable: method involving associated type.
+protocol P7 {
+  typealias Assoc
+  func f7() -> Assoc
+}
+
+// Class A conforms to everything.
+class A : P1, P2, P3, P4, P5, P6, P7 {
+  // P1
+  func f1(a: A) -> Bool { return true }
+
+  // P2
+  var prop2: A {
+    return self
+  }
+
+  // P3
+  subscript (i: Int) -> A {
+  get:
+    return self
+  }
+  
+  // P4
+  subscript (a: A) -> Int { 
+  get: 
+    return 5
+  }
+
+  // P5 
+  func f5() -> A { return self }
+
+  // P6 
+  func f6() -> DynamicSelf { return self }
+
+  // P7
+  func f7() -> Int { return 5 }
+}
+
+// Class B inherits A; gets all of its inheritable conformances.
+class B : A { }
+
+func testB(b: B) {
+  var p1: P1 = b // expected-error{{type 'B' does not conform to protocol 'P1'}}
+  var p2: P2 = b // expected-error{{type 'B' does not conform to protocol 'P2'}}
+  var p3: P3 = b // expected-error{{type 'B' does not conform to protocol 'P3'}}
+  var p4: P4 = b // expected-error{{type 'B' does not conform to protocol 'P4'}}
+  var p5: P5 = b // expected-error{{type 'B' does not conform to protocol 'P5'}}
+  var p6: P6 = b
+  var p7: P7 = b
+}
+
+// Class A5 conforms to P5 in an inheritable manner.
+class A5 : P5 {
+  // P5 
+  func f5() -> DynamicSelf { return self }
+}
+
+// Class B5 inherits A5; gets all of its inheritable conformances.
+class B5 : A5 { }
+
+func testB5(b5: B5) {
+  var p5: P5 = b5 // okay
+}