A Raft of Fixes To Private Dependencies

include/swift/AST/Evaluator.h

@@ -439,7 +439,7 @@ class Evaluator {
                                   !Request::isDependencySink>::type * = nullptr>
   void reportEvaluatedResult(const Request &r,
                              const typename Request::OutputType &o) {
-    recorder.replay(ActiveRequest(r));
+    recorder.replay(activeRequests, ActiveRequest(r));
   }
 
   // Report the result of evaluating a request that is a dependency sink.

include/swift/AST/EvaluatorDependencies.h

@@ -267,10 +267,16 @@ struct DependencyRecorder {
   void realize(const DependencyCollector::Reference &ref);
 
 public:
-  void replay(const swift::ActiveRequest &req);
+  void replay(const llvm::SetVector<swift::ActiveRequest> &stack,
+              const swift::ActiveRequest &req);
   void record(const llvm::SetVector<swift::ActiveRequest> &stack,
               llvm::function_ref<void(DependencyCollector &)> rec);
 
+private:
+  void
+  unionNearestCachedRequest(ArrayRef<swift::ActiveRequest> stack,
+                            const DependencyCollector::ReferenceSet &scratch);
+
 public:
   using ReferenceEnumerator =
       llvm::function_ref<void(const DependencyCollector::Reference &)>;
@@ -298,7 +304,12 @@ struct DependencyRecorder {
   SourceFile *getActiveDependencySourceOrNull() const {
     if (dependencySources.empty())
       return nullptr;
-    return dependencySources.back().getPointer();
+    switch (mode) {
+    case Mode::StatusQuo:
+      return dependencySources.back().getPointer();
+    case Mode::ExperimentalPrivateDependencies:
+      return dependencySources.front().getPointer();
+    }
   }
 
 public:
@@ -331,14 +342,6 @@ struct DependencyRecorder {
   };
 
 private:
-  /// Returns the first dependency source registered with the tracker, or
-  /// \c nullptr if no dependency sources have been registered.
-  SourceFile *getFirstDependencySourceOrNull() const {
-    if (dependencySources.empty())
-      return nullptr;
-    return dependencySources.front().getPointer();
-  }
-
   /// Returns \c true if the scope of the current active source cascades.
   ///
   /// If there is no active scope, the result always cascades.

include/swift/AST/NameLookupRequests.h

@@ -447,7 +447,7 @@ class LookupInModuleRequest
                            QualifiedLookupResult(
                                const DeclContext *, DeclName, NLKind,
                                namelookup::ResolutionKind, const DeclContext *),
-                           RequestFlags::Uncached> {
+                           RequestFlags::Uncached | RequestFlags::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -459,6 +459,11 @@ class LookupInModuleRequest
   evaluate(Evaluator &evaluator, const DeclContext *moduleOrFile, DeclName name,
            NLKind lookupKind, namelookup::ResolutionKind resolutionKind,
            const DeclContext *moduleScopeContext) const;
+
+public:
+  // Incremental dependencies
+  void writeDependencySink(evaluator::DependencyCollector &tracker,
+                           QualifiedLookupResult l) const;
 };
 
 /// Perform \c AnyObject lookup for a given member.

lib/AST/Evaluator.cpp

@@ -446,23 +446,12 @@ void evaluator::DependencyRecorder::record(
     return;
   }
 
-  assert(mode != Mode::StatusQuo);
-  for (const auto &request : stack) {
-    if (!request.isCached()) {
-      continue;
-    }
-
-    auto entry = requestReferences.find_as(request);
-    if (entry == requestReferences.end()) {
-      requestReferences.insert({AnyRequest(request), collector.scratch});
-      continue;
-    }
-
-    entry->second.insert(collector.scratch.begin(), collector.scratch.end());
-  }
+  return unionNearestCachedRequest(stack.getArrayRef(), collector.scratch);
 }
 
-void evaluator::DependencyRecorder::replay(const swift::ActiveRequest &req) {
+void evaluator::DependencyRecorder::replay(
+    const llvm::SetVector<swift::ActiveRequest> &stack,
+    const swift::ActiveRequest &req) {
   assert(!isRecording && "Probably not a good idea to allow nested recording");
 
   auto *source = getActiveDependencySourceOrNull();
@@ -482,6 +471,51 @@ void evaluator::DependencyRecorder::replay(const swift::ActiveRequest &req) {
   for (const auto &ref : entry->second) {
     realize(ref);
   }
+
+  // N.B. This is a particularly subtle detail of the replay unioning step. The
+  // evaluator does not push cached requests onto the active request stack,
+  // so it is possible (and, in fact, quite likely) we'll wind up with an
+  // empty request stack. The remaining troublesome case is when we have a
+  // cached request being run through the uncached path - take the
+  // InterfaceTypeRequest, which involves many component requests, most of which
+  // are themselves cached. In such a case, the active stack will look like
+  //
+  // -> TypeCheckSourceFileRequest
+  // -> ...
+  // -> InterfaceTypeRequest
+  // -> ...
+  // -> UnderlyingTypeRequest
+  //
+  // We want the UnderlyingTypeRequest to union its names into the
+  // InterfaceTypeRequest, and if we were to just start searching the active
+  // stack backwards for a cached request we would find...
+  // the UnderlyingTypeRequest! So, we'll just drop it from consideration.
+  //
+  // We do *not* have to consider this during the recording step because none
+  // of the name lookup requests (or any dependency sinks in general) are
+  // cached. Should this change in the future, we will need to sink this logic
+  // into the union step itself.
+  const size_t d = (!stack.empty() && req == stack.back()) ? 1 : 0;
+  return unionNearestCachedRequest(stack.getArrayRef().drop_back(d),
+                                   entry->second);
+}
+
+void evaluator::DependencyRecorder::unionNearestCachedRequest(
+    ArrayRef<swift::ActiveRequest> stack,
+    const DependencyCollector::ReferenceSet &scratch) {
+  assert(mode != Mode::StatusQuo);
+  auto nearest = std::find_if(stack.rbegin(), stack.rend(),
+                              [](const auto &req){ return req.isCached(); });
+  if (nearest == stack.rend()) {
+    return;
+  }
+
+  auto entry = requestReferences.find_as(*nearest);
+  if (entry == requestReferences.end()) {
+    requestReferences.insert({AnyRequest(*nearest), scratch});
+  } else {
+    entry->second.insert(scratch.begin(), scratch.end());
+  }
 }
 
 using namespace swift;

lib/AST/NameLookupRequests.cpp

@@ -319,6 +319,24 @@ void DirectLookupRequest::writeDependencySink(
   tracker.addUsedMember(desc.DC, desc.Name.getBaseName());
 }
 
+//----------------------------------------------------------------------------//
+// LookupInModuleRequest computation.
+//----------------------------------------------------------------------------//
+
+void LookupInModuleRequest::writeDependencySink(
+    evaluator::DependencyCollector &reqTracker, QualifiedLookupResult l) const {
+  auto *module = std::get<0>(getStorage());
+  auto member = std::get<1>(getStorage());
+  auto *DC = std::get<4>(getStorage());
+
+  // Decline to record lookups outside our module.
+  if (!DC->getParentSourceFile() ||
+      module->getParentModule() != DC->getParentModule()) {
+    return;
+  }
+  reqTracker.addTopLevelName(member.getBaseName());
+}
+
 //----------------------------------------------------------------------------//
 // LookupConformanceInModuleRequest computation.
 //----------------------------------------------------------------------------//

test/Incremental/Verifier/multi-file-private/Inputs/Derived.swift

@@ -1,18 +1,21 @@
 final public class OpenSubclass: OpenBase {} // expected-provides {{OpenSubclass}} expected-private-superclass {{main.OpenBase}}
 // expected-provides {{OpenBase}}
 // expected-private-member {{main.OpenBase.init}}
+// expected-private-member {{main.OpenBase.deinit}}
 // expected-private-member {{main.OpenSubclass.init}}
 // expected-private-member {{main.OpenSubclass.deinit}}
 
 final public class PublicSubclass: PublicBase {} // expected-provides {{PublicSubclass}} expected-private-superclass {{main.PublicBase}}
 // expected-provides {{PublicBase}}
 // expected-private-member {{main.PublicBase.init}}
+// expected-private-member {{main.PublicBase.deinit}}
 // expected-private-member {{main.PublicSubclass.init}}
 // expected-private-member {{main.PublicSubclass.deinit}}
 
 final internal class InternalSubclass: InternalBase {} // expected-provides {{InternalSubclass}} expected-private-superclass {{main.InternalBase}}
 // expected-provides {{InternalBase}}
 // expected-private-member {{main.InternalBase.init}}
+// expected-private-member {{main.InternalBase.deinit}}
 // expected-private-member {{main.InternalSubclass.init}}
 // expected-private-member {{main.InternalSubclass.deinit}}
 

test/Incremental/Verifier/multi-file-private/Inputs/Inner.swift

@@ -0,0 +1,14 @@
+// expected-provides{{Inner}}
+// expected-private-member{{main.Inner.init}}
+public struct Inner {}
+
+// expected-provides{{Foo}}
+public typealias Foo = () -> (Inner)
+
+// expected-provides{{blah}}
+public func blah(foo: Foo) {}
+
+// expected-provides{{defaultFoo}}
+public var defaultFoo: Foo = {
+  return Inner()
+}

test/Incremental/Verifier/multi-file-private/Inputs/UsesInner.swift

@@ -0,0 +1,8 @@
+// expected-provides{{Inner}}
+// expected-provides{{defaultFoo}}
+// expected-provides{{blah}}
+// expected-provides{{Foo}}
+// expected-provides{{??}}
+public func blah(foo: Foo?) {
+  blah(foo: foo ?? defaultFoo)
+}

test/Incremental/Verifier/multi-file-private/main.swift

@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
 // RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
-// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
-// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
+// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
+// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
 
 // N.B. These tests are meant to continue to expand to more and more input files
 // as more kinds of cross-type dependencies are discovered. This will naturally

test/Incremental/Verifier/multi-file/Inputs/Derived.swift

@@ -1,18 +1,21 @@
 final public class OpenSubclass: OpenBase {} // expected-provides {{OpenSubclass}} expected-cascading-superclass {{main.OpenBase}}
 // expected-provides {{OpenBase}}
 // expected-cascading-member {{main.OpenBase.init}}
+// expected-private-member {{main.OpenBase.deinit}}
 // expected-cascading-member {{main.OpenSubclass.init}}
 // expected-private-member {{main.OpenSubclass.deinit}}
 
 final public class PublicSubclass: PublicBase {} // expected-provides {{PublicSubclass}} expected-cascading-superclass {{main.PublicBase}}
 // expected-provides {{PublicBase}}
 // expected-cascading-member {{main.PublicBase.init}}
+// expected-private-member {{main.PublicBase.deinit}}
 // expected-cascading-member {{main.PublicSubclass.init}}
 // expected-private-member {{main.PublicSubclass.deinit}}
 
 final internal class InternalSubclass: InternalBase {} // expected-provides {{InternalSubclass}} expected-cascading-superclass {{main.InternalBase}}
 // expected-provides {{InternalBase}}
 // expected-cascading-member {{main.InternalBase.init}}
+// expected-private-member {{main.InternalBase.deinit}}
 // expected-cascading-member {{main.InternalSubclass.init}}
 // expected-private-member {{main.InternalSubclass.deinit}}
 

test/Incremental/Verifier/multi-file/Inputs/Inner.swift

@@ -0,0 +1,14 @@
+// expected-provides{{Inner}}
+// expected-cascading-member{{main.Inner.init}}
+public struct Inner {}
+
+// expected-provides{{Foo}}
+public typealias Foo = () -> (Inner)
+
+// expected-provides{{blah}}
+public func blah(foo: Foo) {}
+
+// expected-provides{{defaultFoo}}
+public var defaultFoo: Foo = {
+  return Inner()
+}

test/Incremental/Verifier/multi-file/Inputs/UsesInner.swift

@@ -0,0 +1,14 @@
+// FIXME: This dependency ONLY occurs with private dependencies. Otherwise we
+// rely on the interface hash changing to cause this file to be rebuilt, which
+// will *not* work with type fingerprints enabled.
+// See rdar://63984581
+// fixme-provides{{Inner}}
+
+// expected-provides{{defaultFoo}}
+// expected-provides{{blah}}
+// expected-provides{{Optional}}
+// expected-provides{{Foo}}
+// expected-provides{{??}}
+public func blah(foo: Optional<Foo>) {
+  blah(foo: foo ?? defaultFoo)
+}

test/Incremental/Verifier/multi-file/main.swift

@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t)
 // RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
-// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp
-// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -verify-incremental-dependencies @%t.resp
+// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp
+// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -verify-incremental-dependencies @%t.resp
 
 // N.B. These tests are meant to continue to expand to more and more input files
 // as more kinds of cross-type dependencies are discovered. This will naturally