Define Dependency Sinks

Convert most of the name lookup requests and a few other ancillary typechecking requests into dependency sinks.

Some requests are also combined sinks and sources in order to emulate the current scheme, which performs scope changes based on lookup flags. This is generally undesirable, since it means those requests cannot immediately be generalized to a purely context-based scheme because they depend on some client-provided entropy source. In particular, the few callers that are providing the "known private" name lookup flag need to be converted to perform lookups in the appropriate private context.

Clients that are passing "no known dependency" are currently considered universally incorrect and are outside the scope of the compatibility guarantees. This means that request-based dependency tracking registers strictly more edges than manual dependency tracking. It also means that once we fixup the clients that are passing "known private", we can completely remove these name lookup flags.

Finally, some tests had to change to accomodate the new scheme. Currently, we go out of our way to register a dependency edge for extensions that declare protocol conformances. However, we were also asserting in at least one test that extensions without protocol conformances weren't registering dependency edges. This is blatantly incorrect and has been undone now that the request-based scheme is automatically registering this edge.

Author: CodaFi

docs/RequestEvaluator.md

@@ -52,6 +52,11 @@ The request-evaluator contains a cache of any requests that have already been ev
 
 Until then, the request-evaluator lives in a compiler that has mutable ASTs, and will for the foreseeable future. To cope with this, requests can opt for "separate" caching of their results, implementing a simple protocol to query their own cache (`getCachedResult`) and set the value for the cache (`cacheResult`). For now, these functions can directly modify state in the AST, allowing the requests to be mixed with direct mutation of the state. For each request, the intent is to make all state access go through the evaluator, but getting there can be an incremental process.
 
+## Incremental Dependency Tracking
+Request evaluation naturally captures the dependency structure of any given invocation of the compiler frontend. In fact, it captures it so well that the request graph trace generated by a select kind of lookup request can be used to completely recover the information relevant to the Swift compiler's incremental compilation subsystem. For these select *dependency-relevant* requests, we can further subdivide them into so-called *dependency sources* and *dependency sinks*. A dependency source is any (usually high-level) request that introduces a new context under which dependencies can be registered. Currently, these are the requests that operate that the level of individual source files. A dependency sink is any (usually lower-level) request that executes as a sub-computation of a dependency source. Any names that are dependency-relevant, such as the result of a lookup in a particular context, are then registered against trackers in the active dependency source (file). Using this, the evaluator pushes and pops sources and sinks automatically as request evaluation proceeds, and sink requests pair automatically to source requests to write out dependency information.
+
+To define a request as a dependency source, it must implement an accessor for the new active scope (`readDependencySource`). To define a request as a dependency sink, it must implement a function that writes the result of evaluating the request into the current active source (`writeDependencySink`).
+
 ## Open Projects
 
 The request-evaluator is relatively new to the Swift compiler, having been introduced in mid-2018. There are a number of improvements that can be made to the evaluator itself and how it is used in the compiler:

include/swift/AST/Evaluator.h

@@ -464,18 +464,29 @@ class Evaluator {
   }
 
 private:
+  // Report the result of evaluating a request that is not a dependency sink -
+  // which is to say do nothing.
   template <typename Request,
             typename std::enable_if<!Request::isDependencySink>::type * = nullptr>
   void reportEvaluatedResult(const Request &r,
-                             const typename Request::OutputType &o) const {}
+                             const typename Request::OutputType &o) {}
 
+  // Report the result of evaluating a request that is a dependency sink.
   template <typename Request,
             typename std::enable_if<Request::isDependencySink>::type * = nullptr>
   void reportEvaluatedResult(const Request &r,
                              const typename Request::OutputType &o) {
     r.writeDependencySink(*this, o);
   }
 
+  /// If there is an active dependency source, returns its
+  /// \c ReferencedNameTracker. Else, returns \c nullptr.
+  ReferencedNameTracker *getActiveDependencyTracker() const {
+    if (auto *source = getActiveDependencySource())
+      return source->getRequestBasedReferencedNameTracker();
+    return nullptr;
+  }
+
 public:
   /// Returns \c true if the scope of the current active source cascades.
   ///
@@ -496,20 +507,14 @@ class Evaluator {
 
   /// Returns the active dependency's source file, or \c nullptr if no
   /// dependency source is active.
+  ///
+  /// If there is no active scope, the result always cascades.
   SourceFile *getActiveDependencySource() const {
     if (dependencySources.empty())
       return nullptr;
     return dependencySources.back().getPointer();
   }
 
-  /// If there is an active dependency source, returns its
-  /// \c ReferencedNameTracker. Else, returns \c nullptr.
-  ReferencedNameTracker *getActiveDependencyTracker() const {
-    if (auto *source = getActiveDependencySource())
-      return source->getRequestBasedReferencedNameTracker();
-    return nullptr;
-  }
-
 public:
   /// Print the dependencies of the given request as a tree.
   ///

include/swift/AST/EvaluatorDependencies.h

@@ -73,7 +73,7 @@ inline DependencyScope getScopeForAccessLevel(AccessLevel l) {
   }
 }
 
-// A \c DependencySource is a currently defined to be a parent source file and
+// A \c DependencySource is currently defined to be a parent source file and
 // an associated dependency scope.
 //
 // The \c SourceFile instance is an artifact of the current dependency system,

include/swift/AST/NameLookupRequests.h

@@ -18,8 +18,10 @@
 
 #include "swift/AST/SimpleRequest.h"
 #include "swift/AST/ASTTypeIDs.h"
+#include "swift/AST/EvaluatorDependencies.h"
 #include "swift/AST/FileUnit.h"
 #include "swift/AST/Identifier.h"
+#include "swift/AST/NameLookup.h"
 #include "swift/Basic/Statistic.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/TinyPtrVector.h"
@@ -164,9 +166,10 @@ class SuperclassDeclRequest :
 };
 
 class InheritedProtocolsRequest
-    : public SimpleRequest<InheritedProtocolsRequest,
-                           ArrayRef<ProtocolDecl *>(ProtocolDecl *),
-                           CacheKind::SeparatelyCached> {
+    : public SimpleRequest<
+          InheritedProtocolsRequest, ArrayRef<ProtocolDecl *>(ProtocolDecl *),
+          CacheKind::SeparatelyCached | CacheKind::DependencySink |
+              CacheKind::DependencySource> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -178,10 +181,16 @@ class InheritedProtocolsRequest
   evaluate(Evaluator &evaluator, ProtocolDecl *PD) const;
 
 public:
-  // Caching.
+  // Caching
   bool isCached() const { return true; }
   Optional<ArrayRef<ProtocolDecl *>> getCachedResult() const;
-  void cacheResult(ArrayRef<ProtocolDecl *> decls) const;
+  void cacheResult(ArrayRef<ProtocolDecl *> value) const;
+
+public:
+  // Incremental dependencies
+  evaluator::DependencySource readDependencySource(Evaluator &e) const;
+  void writeDependencySink(Evaluator &evaluator,
+                           ArrayRef<ProtocolDecl *> result) const;
 };
 
 /// Requests whether or not this class has designated initializers that are
@@ -208,10 +217,10 @@ class HasMissingDesignatedInitializersRequest :
 };
 
 /// Request the nominal declaration extended by a given extension declaration.
-class ExtendedNominalRequest :
-    public SimpleRequest<ExtendedNominalRequest,
-                         NominalTypeDecl *(ExtensionDecl *),
-                         CacheKind::SeparatelyCached> {
+class ExtendedNominalRequest
+    : public SimpleRequest<
+          ExtendedNominalRequest, NominalTypeDecl *(ExtensionDecl *),
+          CacheKind::SeparatelyCached | CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -227,6 +236,11 @@ class ExtendedNominalRequest :
   bool isCached() const { return true; }
   Optional<NominalTypeDecl *> getCachedResult() const;
   void cacheResult(NominalTypeDecl *value) const;
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(Evaluator &evaluator,
+                           NominalTypeDecl *result) const;
 };
 
 struct SelfBounds {
@@ -292,10 +306,10 @@ class CustomAttrNominalRequest :
 };
 
 /// Finds or synthesizes a destructor for the given class.
-class GetDestructorRequest :
-    public SimpleRequest<GetDestructorRequest,
-                         DestructorDecl *(ClassDecl *),
-                         CacheKind::SeparatelyCached> {
+class GetDestructorRequest
+    : public SimpleRequest<GetDestructorRequest, DestructorDecl *(ClassDecl *),
+                           CacheKind::SeparatelyCached |
+                               CacheKind::DependencySource> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -311,6 +325,10 @@ class GetDestructorRequest :
   bool isCached() const { return true; }
   Optional<DestructorDecl *> getCachedResult() const;
   void cacheResult(DestructorDecl *value) const;
+
+public:
+  // Incremental dependencies.
+  evaluator::DependencySource readDependencySource(Evaluator &) const;
 };
 
 class GenericParamListRequest :
@@ -399,7 +417,8 @@ SourceLoc extractNearestSourceLoc(const UnqualifiedLookupDescriptor &desc);
 class UnqualifiedLookupRequest
     : public SimpleRequest<UnqualifiedLookupRequest,
                            LookupResult(UnqualifiedLookupDescriptor),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached | CacheKind::DependencySource |
+                               CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -409,6 +428,11 @@ class UnqualifiedLookupRequest
   // Evaluation.
   LookupResult evaluate(Evaluator &evaluator,
                         UnqualifiedLookupDescriptor desc) const;
+
+public:
+  // Incremental dependencies
+  evaluator::DependencySource readDependencySource(Evaluator &) const;
+  void writeDependencySink(Evaluator &eval, LookupResult res) const;
 };
 
 using QualifiedLookupResult = SmallVector<ValueDecl *, 4>;
@@ -438,7 +462,7 @@ class AnyObjectLookupRequest
     : public SimpleRequest<AnyObjectLookupRequest,
                            QualifiedLookupResult(const DeclContext *,
                                                  DeclNameRef, NLOptions),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached | CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -449,14 +473,18 @@ class AnyObjectLookupRequest
                                  const DeclContext *dc,
                                  DeclNameRef name,
                                  NLOptions options) const;
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(Evaluator &eval, QualifiedLookupResult l) const;
 };
 
 class ModuleQualifiedLookupRequest
     : public SimpleRequest<ModuleQualifiedLookupRequest,
                            QualifiedLookupResult(const DeclContext *,
                                                  ModuleDecl *, DeclNameRef,
                                                  NLOptions),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached | CacheKind::DependencySource | CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -468,14 +496,20 @@ class ModuleQualifiedLookupRequest
                                  const DeclContext *DC,
                                  ModuleDecl *mod, DeclNameRef name,
                                  NLOptions opts) const;
+
+public:
+  // Incremental dependencies
+  evaluator::DependencySource readDependencySource(Evaluator &) const;
+  void writeDependencySink(Evaluator &eval,
+                           QualifiedLookupResult lookupResult) const;
 };
 
 class QualifiedLookupRequest
     : public SimpleRequest<QualifiedLookupRequest,
                            QualifiedLookupResult(const DeclContext *,
                                                  SmallVector<NominalTypeDecl *, 4>,
                                                  DeclNameRef, NLOptions),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached | CacheKind::DependencySource> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -488,6 +522,10 @@ class QualifiedLookupRequest
            SmallVector<NominalTypeDecl *, 4> decls,
            DeclNameRef name,
            NLOptions opts) const;
+
+public:
+  // Incremental dependencies.
+  evaluator::DependencySource readDependencySource(Evaluator &) const;
 };
 
 /// The input type for a direct lookup request.
@@ -526,7 +564,7 @@ SourceLoc extractNearestSourceLoc(const DirectLookupDescriptor &desc);
 class DirectLookupRequest
     : public SimpleRequest<DirectLookupRequest,
                            TinyPtrVector<ValueDecl *>(DirectLookupDescriptor),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached|CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -536,6 +574,11 @@ class DirectLookupRequest
   // Evaluation.
   TinyPtrVector<ValueDecl *>
   evaluate(Evaluator &evaluator, DirectLookupDescriptor desc) const;
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(Evaluator &evaluator,
+                           TinyPtrVector<ValueDecl *> result) const;
 };
 
 class OperatorLookupDescriptor final {
@@ -598,23 +641,29 @@ template <typename OperatorType>
 class LookupOperatorRequest
     : public SimpleRequest<LookupOperatorRequest<OperatorType>,
                            OperatorType *(OperatorLookupDescriptor),
-                           CacheKind::Cached> {
+                           CacheKind::Cached|CacheKind::DependencySink> {
   using SimpleRequest<LookupOperatorRequest<OperatorType>,
                       OperatorType *(OperatorLookupDescriptor),
-                      CacheKind::Cached>::SimpleRequest;
+                      CacheKind::Cached |
+                          CacheKind::DependencySink>::SimpleRequest;
 
 private:
   friend SimpleRequest<LookupOperatorRequest<OperatorType>,
                        OperatorType *(OperatorLookupDescriptor),
-                       CacheKind::Cached>;
+                       CacheKind::Cached|CacheKind::DependencySink>;
 
   // Evaluation.
-  OperatorType *
-  evaluate(Evaluator &evaluator, OperatorLookupDescriptor desc) const;
+  OperatorType *evaluate(Evaluator &evaluator,
+                         OperatorLookupDescriptor desc) const;
 
 public:
   // Cached.
   bool isCached() const { return true; }
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(Evaluator &evaluator,
+                           OperatorType *o) const;
 };
 
 using LookupPrefixOperatorRequest = LookupOperatorRequest<PrefixOperatorDecl>;
@@ -690,7 +739,7 @@ SourceLoc extractNearestSourceLoc(const LookupConformanceDescriptor &desc);
 class LookupConformanceInModuleRequest
     : public SimpleRequest<LookupConformanceInModuleRequest,
                            ProtocolConformanceRef(LookupConformanceDescriptor),
-                           CacheKind::Uncached> {
+                           CacheKind::Uncached|CacheKind::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -700,6 +749,11 @@ class LookupConformanceInModuleRequest
   // Evaluation.
   ProtocolConformanceRef evaluate(
       Evaluator &evaluator, LookupConformanceDescriptor desc) const;
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(Evaluator &evaluator,
+                           ProtocolConformanceRef result) const;
 };
 
 #define SWIFT_TYPEID_ZONE NameLookup

include/swift/AST/SimpleRequest.h

@@ -270,7 +270,6 @@ class SimpleRequest<Derived, Output(Inputs...), Caching> {
 public:
   constexpr static bool isDependencySource = detail::isDependencySource(Caching);
   constexpr static bool isDependencySink = detail::isDependencySink(Caching);
-  constexpr static bool reportsDependencyReads = isDependencySource || isDependencySink;
 
   using OutputType = Output;