Register operator dependencies on direct lookup requests

Make sure a dependency gets registered when a
direct lookup is fired off. This is necessary in
order to ensure that operator redeclaration
checking records the right dependencies (and any
future logic that might want to perform direct
operator lookups).

In doing so, this commit also removes the
compatibility logic for the old referenced name
tracker that would skip recording dependencies in
certain cases. This should be safe as the new
logic will record strictly more dependencies than
the old logic.

Author: hamishknight

include/swift/AST/NameLookupRequests.h

@@ -649,16 +649,15 @@ template <typename OperatorType>
 class LookupOperatorRequest
     : public SimpleRequest<LookupOperatorRequest<OperatorType>,
                            OperatorType *(OperatorLookupDescriptor),
-                           RequestFlags::Cached|RequestFlags::DependencySink> {
+                           RequestFlags::Cached> {
   using SimpleRequest<LookupOperatorRequest<OperatorType>,
                       OperatorType *(OperatorLookupDescriptor),
-                      RequestFlags::Cached |
-                          RequestFlags::DependencySink>::SimpleRequest;
+                      RequestFlags::Cached>::SimpleRequest;
 
 private:
   friend SimpleRequest<LookupOperatorRequest<OperatorType>,
                        OperatorType *(OperatorLookupDescriptor),
-                       RequestFlags::Cached|RequestFlags::DependencySink>;
+                       RequestFlags::Cached>;
 
   // Evaluation.
   OperatorType *evaluate(Evaluator &evaluator,
@@ -667,11 +666,6 @@ class LookupOperatorRequest
 public:
   // Cached.
   bool isCached() const { return true; }
-
-public:
-  // Incremental dependencies
-  void writeDependencySink(evaluator::DependencyCollector &tracker,
-                           OperatorType *o) const;
 };
 
 using LookupPrefixOperatorRequest = LookupOperatorRequest<PrefixOperatorDecl>;
@@ -685,7 +679,8 @@ class DirectOperatorLookupRequest
     : public SimpleRequest<DirectOperatorLookupRequest,
                            TinyPtrVector<OperatorDecl *>(
                                OperatorLookupDescriptor, OperatorFixity),
-                           RequestFlags::Uncached> {
+                           RequestFlags::Uncached |
+                               RequestFlags::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -695,6 +690,11 @@ class DirectOperatorLookupRequest
   TinyPtrVector<OperatorDecl *>
   evaluate(Evaluator &evaluator, OperatorLookupDescriptor descriptor,
            OperatorFixity fixity) const;
+
+public:
+  // Incremental dependencies.
+  void writeDependencySink(evaluator::DependencyCollector &tracker,
+                           TinyPtrVector<OperatorDecl *> ops) const;
 };
 
 /// Looks up an precedencegroup in a given file or module without looking
@@ -703,7 +703,8 @@ class DirectPrecedenceGroupLookupRequest
     : public SimpleRequest<DirectPrecedenceGroupLookupRequest,
                            TinyPtrVector<PrecedenceGroupDecl *>(
                                OperatorLookupDescriptor),
-                           RequestFlags::Uncached> {
+                           RequestFlags::Uncached |
+                               RequestFlags::DependencySink> {
 public:
   using SimpleRequest::SimpleRequest;
 
@@ -712,6 +713,11 @@ class DirectPrecedenceGroupLookupRequest
 
   TinyPtrVector<PrecedenceGroupDecl *>
   evaluate(Evaluator &evaluator, OperatorLookupDescriptor descriptor) const;
+
+public:
+  // Incremental dependencies.
+  void writeDependencySink(evaluator::DependencyCollector &tracker,
+                           TinyPtrVector<PrecedenceGroupDecl *> groups) const;
 };
 
 class LookupConformanceDescriptor final {

lib/AST/Module.cpp

@@ -1287,48 +1287,28 @@ OperatorType *LookupOperatorRequest<OperatorType>::evaluate(
   return result.hasValue() ? result.getValue() : nullptr;
 }
 
-template <typename OperatorType>
-void LookupOperatorRequest<OperatorType>::writeDependencySink(
-    evaluator::DependencyCollector &reqTracker, OperatorType *o) const {
-  auto &desc = std::get<0>(this->getStorage());
-  auto *FU = desc.fileOrModule.template get<FileUnit *>();
-  auto shouldRegisterDependencyEdge = [&FU](OperatorType *o) -> bool {
-    if (!o)
-      return true;
-
-    auto *topLevelContext = o->getDeclContext()->getModuleScopeContext();
-    return topLevelContext != FU;
-  };
-
-  // FIXME(Evaluator Incremental Dependencies): This is all needlessly complex.
-  // For one, it does not take into account the fact that precedence groups can
-  // be shadowed, and so should be registered regardless of their defining
-  // module. Second, lookups for operators within the file define a valid named
-  // dependency just as much as lookups outside of the current source file.
-  if (!shouldRegisterDependencyEdge(o)) {
-    return;
-  }
-
-  reqTracker.addTopLevelName(desc.name);
-}
-
 #define LOOKUP_OPERATOR(Kind)                                                  \
   Kind##Decl *ModuleDecl::lookup##Kind(Identifier name, SourceLoc loc) {       \
     auto result = lookupOperatorDeclForName<Kind##Decl>(                       \
         this, loc, name, /*isCascading*/ false);                               \
     return result ? *result : nullptr;                                         \
   }                                                                            \
   template Kind##Decl *LookupOperatorRequest<Kind##Decl>::evaluate(            \
-      Evaluator &e, OperatorLookupDescriptor) const;                           \
-  template void LookupOperatorRequest<Kind##Decl>::writeDependencySink(        \
-      evaluator::DependencyCollector &, Kind##Decl *) const;
+      Evaluator &e, OperatorLookupDescriptor) const;
 
 LOOKUP_OPERATOR(PrefixOperator)
 LOOKUP_OPERATOR(InfixOperator)
 LOOKUP_OPERATOR(PostfixOperator)
 LOOKUP_OPERATOR(PrecedenceGroup)
 #undef LOOKUP_OPERATOR
 
+void DirectOperatorLookupRequest::writeDependencySink(
+    evaluator::DependencyCollector &reqTracker,
+    TinyPtrVector<OperatorDecl *> ops) const {
+  auto &desc = std::get<0>(getStorage());
+  reqTracker.addTopLevelName(desc.name);
+}
+
 TinyPtrVector<OperatorDecl *>
 DirectOperatorLookupRequest::evaluate(Evaluator &evaluator,
                                       OperatorLookupDescriptor descriptor,
@@ -1357,6 +1337,13 @@ void SourceFile::lookupOperatorDirect(
   getCache().lookupOperator(name, fixity, results);
 }
 
+void DirectPrecedenceGroupLookupRequest::writeDependencySink(
+    evaluator::DependencyCollector &reqTracker,
+    TinyPtrVector<PrecedenceGroupDecl *> groups) const {
+  auto &desc = std::get<0>(getStorage());
+  reqTracker.addTopLevelName(desc.name);
+}
+
 TinyPtrVector<PrecedenceGroupDecl *>
 DirectPrecedenceGroupLookupRequest::evaluate(
     Evaluator &evaluator, OperatorLookupDescriptor descriptor) const {