Merge pull request #41146 from AnthonyLatsis/val-witness-res-untied

ConformanceChecker: Resolve value witnesses unless they depend on invalid type witnesses

Author: AnthonyLatsis

lib/Sema/CSDiagnostics.cpp

@@ -3149,8 +3149,9 @@ bool ContextualFailure::tryProtocolConformanceFixIt(
           ProtocolConformanceState::Incomplete, /*isUnchecked=*/false);
       ConformanceChecker checker(getASTContext(), &conformance,
                                  missingWitnesses);
-      checker.resolveValueWitnesses();
+      // Type witnesses must be resolved first.
       checker.resolveTypeWitnesses();
+      checker.resolveValueWitnesses();
     }
 
     for (auto decl : missingWitnesses) {

lib/Sema/TypeCheckProtocol.cpp

@@ -4847,12 +4847,6 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
 void ConformanceChecker::ensureRequirementsAreSatisfied() {
   Conformance->finishSignatureConformances();
   auto proto = Conformance->getProtocol();
-
-  if (CheckedRequirementSignature)
-    return;
-
-  CheckedRequirementSignature = true;
-
   auto &diags = proto->getASTContext().Diags;
 
   auto DC = Conformance->getDeclContext();
@@ -4935,6 +4929,71 @@ void ConformanceChecker::ensureRequirementsAreSatisfied() {
 
 #pragma mark Protocol conformance checking
 
+/// Determine whether mapping the interface type of the given protocol non-type
+/// requirement into the context of the given conformance produces a well-formed
+/// type.
+static bool
+hasInvalidTypeInConformanceContext(const ValueDecl *requirement,
+                                   NormalProtocolConformance *conformance) {
+  assert(!isa<TypeDecl>(requirement));
+  assert(requirement->getDeclContext()->getSelfProtocolDecl() ==
+         conformance->getProtocol());
+
+  // FIXME: getInterfaceType() on properties returns contextual types that have
+  // been mapped out of context, but mapTypeOutOfContext() does not reconstitute
+  // type parameters that were substituted with concrete types. Instead,
+  // patterns should be refactored to use interface types, at least if they
+  // appear in type contexts.
+  auto interfaceTy = requirement->getInterfaceType();
+
+  // Skip the curried 'self' parameter.
+  if (requirement->hasCurriedSelf())
+    interfaceTy = interfaceTy->castTo<AnyFunctionType>()->getResult();
+
+  // For subscripts, build a regular function type to skip walking generic
+  // requirements.
+  if (auto *gft = interfaceTy->getAs<GenericFunctionType>()) {
+    interfaceTy = FunctionType::get(gft->getParams(), gft->getResult(),
+                                    gft->getExtInfo());
+  }
+
+  if (!interfaceTy->hasTypeParameter())
+    return false;
+
+  const auto subs = SubstitutionMap::getProtocolSubstitutions(
+      conformance->getProtocol(),
+      conformance->getDeclContext()->mapTypeIntoContext(conformance->getType()),
+      ProtocolConformanceRef(conformance));
+
+  class Walker final : public TypeWalker {
+    const SubstitutionMap &Subs;
+    const ProtocolDecl *Proto;
+
+  public:
+    explicit Walker(const SubstitutionMap &Subs, const ProtocolDecl *Proto)
+        : Subs(Subs), Proto(Proto) {}
+
+    Action walkToTypePre(Type ty) override {
+      if (!ty->hasTypeParameter())
+        return Action::SkipChildren;
+
+      auto *const dmt = ty->getAs<DependentMemberType>();
+      if (!dmt)
+        return Action::Continue;
+
+      // We only care about 'Self'-rooted type parameters.
+      if (!dmt->getRootGenericParam()->isEqual(Proto->getSelfInterfaceType()))
+        return Action::SkipChildren;
+
+      if (ty.subst(Subs)->hasError())
+        return Action::Stop;
+      return Action::SkipChildren;
+    }
+  };
+
+  return interfaceTy.walk(Walker(subs, conformance->getProtocol()));
+}
+
 void ConformanceChecker::resolveValueWitnesses() {
   for (auto member : Proto->getMembers()) {
     auto requirement = dyn_cast<ValueDecl>(member);
@@ -5086,10 +5145,6 @@ void ConformanceChecker::resolveValueWitnesses() {
       continue;
     }
 
-    // If this is an accessor for a storage decl, ignore it.
-    if (isa<AccessorDecl>(requirement))
-      continue;
-
     // If this requirement is part of a pair of imported async requirements,
     // where one has already been witnessed, we can skip it.
     //
@@ -5106,6 +5161,13 @@ void ConformanceChecker::resolveValueWitnesses() {
       continue;
     }
 
+    // Try substituting into the requirement's interface type. If we fail,
+    // either a generic requirement was not satisfied or we tripped on an
+    // invalid type witness, and there's no point in resolving a witness.
+    if (hasInvalidTypeInConformanceContext(requirement, Conformance)) {
+      continue;
+    }
+
     // Try to resolve the witness.
     switch (resolveWitnessTryingAllStrategies(requirement)) {
     case ResolveWitnessResult::Success:
@@ -5193,14 +5255,6 @@ void ConformanceChecker::checkConformance(MissingWitnessDiagnosisKind Kind) {
   // Diagnose missing type witnesses for now.
   diagnoseMissingWitnesses(Kind);
 
-  // If we complain about any associated types, there is no point in continuing.
-  // FIXME: Not really true. We could check witnesses that don't involve the
-  // failed associated types.
-  if (AlreadyComplained) {
-    Conformance->setInvalid();
-    return;
-  }
-
   // Diagnose missing value witnesses later.
   SWIFT_DEFER { diagnoseMissingWitnesses(Kind); };
 

lib/Sema/TypeCheckProtocol.h

@@ -716,9 +716,6 @@ class ConformanceChecker : public WitnessChecker {
   /// Whether we've already complained about problems with this conformance.
   bool AlreadyComplained = false;
 
-  /// Whether we checked the requirement signature already.
-  bool CheckedRequirementSignature = false;
-
   /// Mapping from Objective-C methods to the set of requirements within this
   /// protocol that have the same selector and instance/class designation.
   llvm::SmallDenseMap<ObjCMethodKey, TinyPtrVector<AbstractFunctionDecl *>, 4>
@@ -791,6 +788,10 @@ class ConformanceChecker : public WitnessChecker {
   ResolveWitnessResult resolveTypeWitnessViaLookup(
                          AssociatedTypeDecl *assocType);
 
+  /// Check whether all of the protocol's generic requirements are satisfied by
+  /// the chosen type witnesses.
+  void ensureRequirementsAreSatisfied();
+
   /// Diagnose or defer a diagnostic, as appropriate.
   ///
   /// \param requirement The requirement with which this diagnostic is
@@ -849,10 +850,6 @@ class ConformanceChecker : public WitnessChecker {
   /// directly as possible.
   void resolveSingleTypeWitness(AssociatedTypeDecl *assocType);
 
-  /// Check all of the protocols requirements are actually satisfied by a
-  /// the chosen type witnesses.
-  void ensureRequirementsAreSatisfied();
-
   /// Check the entire protocol conformance, ensuring that all
   /// witnesses are resolved and emitting any diagnostics.
   void checkConformance(MissingWitnessDiagnosisKind Kind);

test/decl/protocol/conforms/failure.swift

@@ -131,3 +131,119 @@ extension UInt32: ExpressibleByStringLiteral {}
 // the type actually conforming), do not crash when failing to find
 // the associated witness type.
 let diagnose: UInt32 = "reta"
+
+// Test that we attempt to resolve a value witness unless mapping its interface
+// type into the conformance context produces an invalid type.
+protocol P7 {
+  associatedtype A: Sequence // expected-note {{protocol requires nested type 'A'; do you want to add it?}}
+
+  subscript(subscript1 _: A) -> Never { get }
+  subscript<T>(subscript2 _: T) -> Never where T: Sequence, T.Element == A { get }
+  // expected-note@-1 {{protocol requires subscript with type '<T> (subscript2: T) -> Never'; do you want to add a stub?}}
+
+  func method1(_: A)
+  func method2() -> A.Element
+  func method3<T>(_: T) where T: Sequence, T.Element == A
+  // expected-note@-1 {{protocol requires function 'method3' with type '<T> (T) -> ()'; do you want to add a stub?}}
+  func method4(_: Never) // expected-note {{protocol requires function 'method4' with type '(Never) -> ()'; do you want to add a stub?}}
+}
+do {
+  struct Conformer: P7 {} // expected-error {{type 'Conformer' does not conform to protocol 'P7'}}
+}
+
+protocol P8 {
+  associatedtype A: Sequence where A.Element == Never
+
+  func method1(_: A) // expected-note {{protocol requires function 'method1' with type '(Conformer.A) -> ()' (aka '(Array<Bool>) -> ()'); do you want to add a stub?}}
+  func method2() -> A.Element // expected-note {{protocol requires function 'method2()' with type '() -> Bool'; do you want to add a stub?}}
+  func method3<T>(_: T) where T: Sequence, T.Element == A
+  // expected-note@-1 {{protocol requires function 'method3' with type '<T> (T) -> ()'; do you want to add a stub?}}
+  func method4(_: Never) // expected-note {{protocol requires function 'method4' with type '(Never) -> ()'; do you want to add a stub?}}
+}
+do {
+  struct Conformer: P8 {
+    // expected-error@-1 {{type 'Conformer' does not conform to protocol 'P8'}}
+    // expected-error@-2 {{'P8' requires the types 'Bool' and 'Never' be equivalent}}
+    // expected-note@-3 {{requirement specified as 'Self.A.Element' == 'Never' [with Self = Conformer]}}
+    typealias A = Array<Bool>
+  }
+}
+
+protocol P9a {
+  associatedtype A
+}
+protocol P9b: P9a where A: Sequence {
+  subscript(subscript1 _: A.Element) -> Never { get }
+  subscript<T>(subscript2 _: T) -> Never where T: Sequence, T.Element == A.Element { get }
+  // expected-note@-1 {{protocol requires subscript with type '<T> (subscript2: T) -> Never'; do you want to add a stub?}}
+
+  func method1(_: A) // expected-note {{protocol requires function 'method1' with type '(Conformer.A) -> ()' (aka '(Bool) -> ()'); do you want to add a stub?}}
+  func method2() -> A.Element
+  func method3<T>(_: T) where T: Sequence, T.Element == A
+  // expected-note@-1 {{protocol requires function 'method3' with type '<T> (T) -> ()'; do you want to add a stub?}}
+  func method4(_: Never) // expected-note {{protocol requires function 'method4' with type '(Never) -> ()'; do you want to add a stub?}}
+}
+do {
+  struct Conformer: P9b {
+    // expected-error@-1 {{type 'Conformer' does not conform to protocol 'P9b'}}
+    // expected-error@-2 {{type 'Conformer.A' (aka 'Bool') does not conform to protocol 'Sequence'}}
+    typealias A = Bool
+  }
+}
+
+protocol P10a {
+  associatedtype A
+}
+protocol P10b: P10a where A: Sequence, A.Element == Never {
+  func method1(_: A) // expected-note {{protocol requires function 'method1' with type '(Conformer.A) -> ()' (aka '(Array<Bool>) -> ()'); do you want to add a stub?}}
+  func method2() -> A.Element // expected-note {{protocol requires function 'method2()' with type '() -> Bool'; do you want to add a stub?}}
+  func method3<T>(_: T) where T: Sequence, T.Element == A
+  // expected-note@-1 {{protocol requires function 'method3' with type '<T> (T) -> ()'; do you want to add a stub?}}
+  func method4(_: Never) // expected-note {{protocol requires function 'method4' with type '(Never) -> ()'; do you want to add a stub?}}
+}
+do {
+  struct Conformer: P10b {
+    // expected-error@-1 {{type 'Conformer' does not conform to protocol 'P10b'}}
+    // expected-error@-2 {{'P10b' requires the types 'Bool' and 'Never' be equivalent}}
+    // expected-note@-3 {{requirement specified as 'Self.A.Element' == 'Never' [with Self = Conformer]}}
+    typealias A = Array<Bool>
+  }
+}
+
+protocol P11 {
+  associatedtype A: Equatable
+  // FIXME: Should not resolve witness for 'method', but Type::subst doesn't care
+  // about conditional requirements when querying type witnesses.
+  // expected-note@+1 {{protocol requires function 'method()' with type '() -> Conformer.A' (aka '() -> Array<P11>'); do you want to add a stub?}}
+  func method() -> A
+}
+do {
+  struct Conformer: P11 {
+    // expected-error@-1 {{type 'Conformer' does not conform to protocol 'P11'}}
+    // expected-error@-2 {{type 'P11' does not conform to protocol 'Equatable'}} // FIXME: Crappy diagnostics
+    // expected-error@-3 {{'P11' requires that 'P11' conform to 'Equatable'}}
+    // expected-note@-4 {{requirement specified as 'P11' : 'Equatable'}}
+    // expected-note@-5 {{requirement from conditional conformance of 'Conformer.A' (aka 'Array<P11>') to 'Equatable'}}
+    typealias A = Array<any P11>
+  }
+}
+
+protocol P12 {
+  associatedtype A: Sequence where A.Element == Never // expected-note {{protocol requires nested type 'A'; do you want to add it?}}
+  // FIXME: Should not resolve witness for 'prop', but getInterfaceType() returns
+  // '(Self) -> Never' instead of '(Self) -> Self.A.Element', and the invalid
+  // type parameter is never found (see 'hasInvalidTypeInConformanceContext').
+  // This happens because getInterfaceType() on properties returns contextual
+  // types that have been mapped out of context, but mapTypeOutOfContext() does
+  // not reconstitute type parameters that were substituted with concrete types.
+  // Instead, patterns should be refactored to use interface types, at least if
+  // they appear in type contexts.
+  //
+  // expected-note@+1 {{protocol requires property 'prop' with type '(Conformer) -> Never'; do you want to add a stub?}}
+  var prop: (Self) -> A.Element { get }
+}
+do {
+  struct Conformer: P12 { // expected-error {{type 'Conformer' does not conform to protocol 'P12'}}
+    typealias A = Bool // expected-note {{possibly intended match 'Conformer.A' (aka 'Bool') does not conform to 'Sequence'}}
+  }
+}

test/decl/protocol/conforms/fixit_stub.swift

@@ -162,17 +162,18 @@ protocol ProtocolChain2 : ProtocolChain1 {
 class Adopter15 : ProtocolChain2 {} //expected-error {{type 'Adopter15' does not conform to protocol 'ProtocolChain2'}} expected-error {{type 'Adopter15' does not conform to protocol 'ProtocolChain1'}}
 
 protocol ProtocolParallel1 {
-  associatedtype Foo1 // expected-note {{protocol requires nested type 'Foo1'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  associatedtype Foo2 // expected-note {{protocol requires nested type 'Foo2'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  associatedtype Foo3 // expected-note {{protocol requires nested type 'Foo3'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  func Foo4()
+  associatedtype Foo1 // expected-note {{protocol requires nested type 'Foo1'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  associatedtype Foo2 // expected-note {{protocol requires nested type 'Foo2'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  associatedtype Foo3 // expected-note {{protocol requires nested type 'Foo3'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  // FIXME: Why do we add stubs for all missing requirements when the note implies a single one?
+  func Foo4() // expected-note {{protocol requires function 'Foo4()' with type '() -> ()'; do you want to add a stub?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
 }
 
 protocol ProtocolParallel2 {
-  associatedtype Bar1 // expected-note {{protocol requires nested type 'Bar1'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  associatedtype Bar2 // expected-note {{protocol requires nested type 'Bar2'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  associatedtype Bar3 // expected-note {{protocol requires nested type 'Bar3'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
-  func Bar4()
+  associatedtype Bar1 // expected-note {{protocol requires nested type 'Bar1'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  associatedtype Bar2 // expected-note {{protocol requires nested type 'Bar2'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  associatedtype Bar3 // expected-note {{protocol requires nested type 'Bar3'; do you want to add it?}}{{57-57=\n    typealias Foo1 = <#type#>\n\n    typealias Foo2 = <#type#>\n\n    typealias Foo3 = <#type#>\n\n    func Foo4() {\n        <#code#>\n    \}\n\n    typealias Bar1 = <#type#>\n\n    typealias Bar2 = <#type#>\n\n    typealias Bar3 = <#type#>\n}}
+  func Bar4() // expected-note {{protocol requires function 'Bar4()' with type '() -> ()'; do you want to add a stub?}}{{57-57=\n    func Bar4() {\n        <#code#>\n    \}\n}}
 }
 
 class Adopter16 : ProtocolParallel1, ProtocolParallel2 {} // expected-error {{type 'Adopter16' does not conform to protocol 'ProtocolParallel1'}} expected-error {{type 'Adopter16' does not conform to protocol 'ProtocolParallel2'}}