Requestify Witness Resolution

Witness matching is a source of a lot of ad-hoc cycles, and mixes the
logic that performs resolution, caching, validation, and cycle detection into one
place.  To make matters worse, some checkers kick off other checks in
order to cache work for further declarations, and access an internal
cache on their subject conformance for many requirements at once, or
sometimes just one requirement.

None of this fits into the request evaluator's central view of the
caching.  This is further evidenced by the fact that if you attempt to
move the caching step into the evaluator, it overcaches the same
witness and trips asserts.

As a start, define requests for the resolution steps, and flush some
hacks around forcing witness resolution. The caching logic is mostly
untouched (the requests don't actually cache anything), but some cycle
breaking is now handled in the evaluator itself.  Once witness matching
has been refactored to cache with the evaluator, all of these hacks can
go away.

My urge to destroy the LazyResolver outweighs the compromises here.

Author: CodaFi

include/swift/AST/ASTTypeIDZone.def

@@ -25,6 +25,8 @@ SWIFT_TYPEID(Requirement)
 SWIFT_TYPEID(ResilienceExpansion)
 SWIFT_TYPEID(Type)
 SWIFT_TYPEID(TypePair)
+SWIFT_TYPEID(TypeWitnessAndDecl)
+SWIFT_TYPEID(Witness)
 SWIFT_TYPEID_NAMED(ConstructorDecl *, ConstructorDecl)
 SWIFT_TYPEID_NAMED(CustomAttr *, CustomAttr)
 SWIFT_TYPEID_NAMED(Decl *, Decl)

include/swift/AST/ASTTypeIDs.h

@@ -51,9 +51,11 @@ enum class ResilienceExpansion : unsigned;
 class Type;
 class ValueDecl;
 class VarDecl;
+class Witness;
 class TypeAliasDecl;
 class Type;
 struct TypePair;
+struct TypeWitnessAndDecl;
 enum class AncestryFlags : uint8_t;
 
 // Define the AST type zone (zone 1)

include/swift/AST/Decl.h

@@ -7289,6 +7289,11 @@ inline void simple_display(llvm::raw_ostream &out,
   simple_display(out, static_cast<const Decl *>(decl));
 }
 
+inline void simple_display(llvm::raw_ostream &out,
+                           const AssociatedTypeDecl *decl) {
+  simple_display(out, static_cast<const Decl *>(decl));
+}
+
 /// Display GenericContext.
 ///
 /// The template keeps this sorted down in the overload set relative to the

include/swift/AST/LazyResolver.h

@@ -43,16 +43,6 @@ class LazyResolver {
 public:
   virtual ~LazyResolver();
 
-  /// Resolve the type witnesses for the given associated type within the given
-  /// protocol conformance.
-  virtual void resolveTypeWitness(const NormalProtocolConformance *conformance,
-                                  AssociatedTypeDecl *assocType) = 0;
-
-  /// Resolve the witness for the given non-type requirement within
-  /// the given protocol conformance.
-  virtual void resolveWitness(const NormalProtocolConformance *conformance,
-                              ValueDecl *requirement) = 0;
-
   /// Resolve any implicitly-declared constructors within the given nominal.
   virtual void resolveImplicitConstructors(NominalTypeDecl *nominal) = 0;
 

include/swift/AST/ProtocolConformance.h

@@ -52,7 +52,7 @@ typedef llvm::DenseMap<ValueDecl *, Witness> WitnessMap;
 
 /// Map from associated type requirements to the corresponding type and
 /// the type declaration that was used to satisfy the requirement.
-typedef llvm::DenseMap<AssociatedTypeDecl *, std::pair<Type, TypeDecl*>>
+typedef llvm::DenseMap<AssociatedTypeDecl *, TypeWitnessAndDecl>
   TypeWitnessMap;
 
 /// Describes the kind of protocol conformance structure used to encode
@@ -157,7 +157,7 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance {
 
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
-  std::pair<Type, TypeDecl *>
+  TypeWitnessAndDecl
   getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
                         SubstOptions options=None) const;
 
@@ -182,7 +182,7 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance {
         continue;
 
       const auto &TWInfo = getTypeWitnessAndDecl(assocTypeReq);
-      if (f(assocTypeReq, TWInfo.first, TWInfo.second))
+      if (f(assocTypeReq, TWInfo.getWitnessType(), TWInfo.getWitnessDecl()))
         return true;
     }
 
@@ -404,6 +404,9 @@ class RootProtocolConformance : public ProtocolConformance {
 class NormalProtocolConformance : public RootProtocolConformance,
                                   public llvm::FoldingSetNode
 {
+  friend class ValueWitnessRequest;
+  friend class TypeWitnessRequest;
+
   /// The protocol being conformed to and its current state.
   llvm::PointerIntPair<ProtocolDecl *, 2, ProtocolConformanceState>
     ProtocolAndState;
@@ -588,10 +591,13 @@ class NormalProtocolConformance : public RootProtocolConformance,
 
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
-  std::pair<Type, TypeDecl *>
+  TypeWitnessAndDecl
   getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
                         SubstOptions options=None) const;
 
+  TypeWitnessAndDecl
+  getTypeWitnessUncached(AssociatedTypeDecl *requirement) const;
+
   /// Determine whether the protocol conformance has a type witness for the
   /// given associated type.
   bool hasTypeWitness(AssociatedTypeDecl *assocType) const;
@@ -610,6 +616,8 @@ class NormalProtocolConformance : public RootProtocolConformance,
   /// Retrieve the value witness corresponding to the given requirement.
   Witness getWitness(ValueDecl *requirement) const;
 
+  Witness getWitnessUncached(ValueDecl *requirement) const;
+
   /// Determine whether the protocol conformance has a witness for the given
   /// requirement.
   bool hasWitness(ValueDecl *requirement) const {
@@ -641,7 +649,7 @@ class NormalProtocolConformance : public RootProtocolConformance,
   /// Determine whether the witness for the given type requirement
   /// is the default definition.
   bool usesDefaultDefinition(AssociatedTypeDecl *requirement) const {
-    TypeDecl *witnessDecl = getTypeWitnessAndDecl(requirement).second;
+    TypeDecl *witnessDecl = getTypeWitnessAndDecl(requirement).getWitnessDecl();
     if (witnessDecl)
       return witnessDecl->isImplicit();
     // Conservatively assume it does not.
@@ -713,7 +721,7 @@ class SelfProtocolConformance : public RootProtocolConformance {
     llvm_unreachable("self-conformances never have associated types");
   }
 
-  std::pair<Type, TypeDecl *>
+  TypeWitnessAndDecl
   getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
                         SubstOptions options=None) const {
     llvm_unreachable("self-conformances never have associated types");
@@ -859,7 +867,7 @@ class SpecializedProtocolConformance : public ProtocolConformance,
 
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
-  std::pair<Type, TypeDecl *>
+  TypeWitnessAndDecl
   getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
                         SubstOptions options=None) const;
 
@@ -971,7 +979,7 @@ class InheritedProtocolConformance : public ProtocolConformance,
 
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
-  std::pair<Type, TypeDecl *>
+  TypeWitnessAndDecl
   getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
                         SubstOptions options=None) const {
     return InheritedConformance->getTypeWitnessAndDecl(assocType, options);
@@ -1015,6 +1023,8 @@ inline bool ProtocolConformance::hasWitness(ValueDecl *requirement) const {
   return getRootConformance()->hasWitness(requirement);
 }
 
+void simple_display(llvm::raw_ostream &out, const ProtocolConformance *conf);
+
 } // end namespace swift
 
 #endif // LLVM_SWIFT_AST_PROTOCOLCONFORMANCE_H

include/swift/AST/TypeCheckRequests.h

@@ -41,6 +41,8 @@ class RequirementRepr;
 class SpecializeAttr;
 class TypeAliasDecl;
 struct TypeLoc;
+class Witness;
+struct TypeWitnessAndDecl;
 class ValueDecl;
 enum class OpaqueReadOwnership: uint8_t;
 class StorageImplInfo;
@@ -1668,6 +1670,51 @@ class InheritsSuperclassInitializersRequest
   void cacheResult(bool value) const;
 };
 
+class TypeWitnessRequest
+    : public SimpleRequest<TypeWitnessRequest,
+                           TypeWitnessAndDecl(NormalProtocolConformance *,
+                                              AssociatedTypeDecl *),
+                           CacheKind::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<TypeWitnessAndDecl>
+  evaluate(Evaluator &evaluator, NormalProtocolConformance *conformance,
+           AssociatedTypeDecl *ATD) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+  Optional<TypeWitnessAndDecl> getCachedResult() const;
+  void cacheResult(TypeWitnessAndDecl value) const;
+};
+
+class ValueWitnessRequest
+    : public SimpleRequest<ValueWitnessRequest,
+                           Witness(NormalProtocolConformance *, ValueDecl *),
+                           CacheKind::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  llvm::Expected<Witness> evaluate(Evaluator &evaluator,
+                                   NormalProtocolConformance *conformance,
+                                   ValueDecl *VD) const;
+
+public:
+  // Separate caching.
+  bool isCached() const { return true; }
+  Optional<Witness> getCachedResult() const;
+  void cacheResult(Witness value) const;
+};
+
 // Allow AnyValue to compare two Type values, even though Type doesn't
 // support ==.
 template<>

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -68,6 +68,8 @@ SWIFT_REQUEST(TypeChecker, InheritedTypeRequest,
               Type(llvm::PointerUnion<TypeDecl *, ExtensionDecl *>, unsigned,
                    TypeResolutionStage),
               SeparatelyCached, HasNearestLocation)
+SWIFT_REQUEST(TypeChecker, InheritsSuperclassInitializersRequest,
+              bool(ClassDecl *), SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, InitKindRequest,
               CtorInitializerKind(ConstructorDecl *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, InterfaceTypeRequest,
@@ -180,5 +182,10 @@ SWIFT_REQUEST(TypeChecker, HasDefaultInitRequest,
               bool(NominalTypeDecl *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, SynthesizeDefaultInitRequest,
               ConstructorDecl *(NominalTypeDecl *), Cached, NoLocationInfo)
-SWIFT_REQUEST(TypeChecker, InheritsSuperclassInitializersRequest,
-              bool(ClassDecl *), SeparatelyCached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, TypeWitnessRequest,
+              TypeWitnessAndDecl(NormalProtocolConformance *,
+                                 AssociatedTypeDecl *),
+              SeparatelyCached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, ValueWitnessRequest,
+              Witness(NormalProtocolConformance *, ValueDecl *),
+              SeparatelyCached, NoLocationInfo)

include/swift/AST/Witness.h

@@ -188,6 +188,40 @@ class Witness {
   void dump(llvm::raw_ostream &out) const;
 };
 
+struct TypeWitnessAndDecl {
+  Type witnessType;
+  TypeDecl *witnessDecl = nullptr;
+
+  TypeWitnessAndDecl() = default;
+  TypeWitnessAndDecl(Type ty, TypeDecl *decl)
+    : witnessType(ty), witnessDecl(decl) {}
+
+public:
+  Type getWitnessType() const {
+    return witnessType;
+  }
+
+  TypeDecl *getWitnessDecl() const {
+    return witnessDecl;
+  }
+
+  friend llvm::hash_code hash_value(const TypeWitnessAndDecl &owner) {
+    return llvm::hash_combine(owner.witnessType,
+                              owner.witnessDecl);
+  }
+
+  friend bool operator==(const TypeWitnessAndDecl &lhs,
+                         const TypeWitnessAndDecl &rhs) {
+    return lhs.witnessType->isEqual(rhs.witnessType) &&
+           lhs.witnessDecl == rhs.witnessDecl;
+  }
+
+  friend bool operator!=(const TypeWitnessAndDecl &lhs,
+                         const TypeWitnessAndDecl &rhs) {
+    return !(lhs == rhs);
+  }
+};
+
 } // end namespace swift
 
 #endif // SWIFT_AST_WITNESS_H

lib/AST/ASTVerifier.cpp

@@ -2645,7 +2645,8 @@ class Verifier : public ASTWalker {
 
           // Make sure that the replacement type only uses archetypes allowed
           // in the context where the normal conformance exists.
-          auto replacementType = normal->getTypeWitness(assocType);
+          auto replacementType =
+              normal->getTypeWitnessUncached(assocType).getWitnessType();
           Verifier(M, normal->getDeclContext())
             .verifyChecked(replacementType);
           continue;
@@ -2677,7 +2678,7 @@ class Verifier : public ASTWalker {
           }
 
           // Check the witness substitutions.
-          const auto &witness = normal->getWitness(req);
+          const auto &witness = normal->getWitnessUncached(req);
 
           if (auto *genericEnv = witness.getSyntheticEnvironment())
             Generics.push_back(genericEnv->getGenericSignature());