restore completion results order stability

Author: AnthonyLatsis

lib/AST/LookupVisibleDecls.cpp

@@ -473,67 +473,35 @@ lookupVisibleMemberDeclsImpl(Type BaseTy, VisibleDeclConsumer &Consumer,
                              GenericSignatureBuilder *GSB,
                              VisitedSet &Visited);
 
+// Filters out restated declarations from a protocol hierarchy
+// or equivalent requirements from protocol composition types.
 class RestateFilteringConsumer : public VisibleDeclConsumer {
-  VisibleDeclConsumer &parentConsumer;
   LazyResolver *resolver;
 
   using FoundDecl = std::pair<ValueDecl*, DeclVisibilityKind>;
   using NameAndType = std::pair<DeclName, CanType>;
 
   llvm::DenseMap<DeclName, FoundDecl> foundVars;
   llvm::DenseMap<NameAndType, FoundDecl> foundFuncs;
-
-public:
-  RestateFilteringConsumer(VisibleDeclConsumer &parentConsumer,
-                           Type baseTy, const DeclContext *DC,
-                           LazyResolver *resolver)
-  : parentConsumer(parentConsumer), resolver(resolver) {
-    assert(DC && baseTy && !baseTy->hasLValueType());
-  }
-
-  void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
-    assert(VD);
-    // If this isn't a protocol context, hand over the decl
-    // to the parent consumer.
-    if (!isa<ProtocolDecl>(VD->getDeclContext())) {
-      parentConsumer.foundDecl(VD, Reason);
-      return;
-    }
-    if (resolver)
-      resolver->resolveDeclSignature(VD);
-
-    if (!VD->hasInterfaceType()) {
-      parentConsumer.foundDecl(VD, Reason);
-      return;
-    }
-    // Do not feed anything to the parent consumer if we found a valid
-    // new declaration: there might be another conflicting one with a
-    // higher access level that we want to prioritize later.
-    if (auto GFT = VD->getInterfaceType()->getAs<GenericFunctionType>()) {
-      auto type = stripSelfRequirementsIfNeeded(VD, GFT);
-      NameAndType nameAndType = {VD->getFullName(), type};
-
-      if ((foundFuncs.find(nameAndType) == foundFuncs.end()) ||
-          (foundFuncs[nameAndType].first->getFormalAccess() <
-           VD->getFormalAccess()))
-        foundFuncs[nameAndType] = {VD, Reason};
-      return;
+  llvm::MapVector<ValueDecl*, DeclVisibilityKind> declsToReport;
+
+  template <typename K>
+  void addDecl(llvm::DenseMap<K, FoundDecl> &Map, K Key, FoundDecl FD) {
+    // Add the declaration if we haven't found an equivalent yet, otherwise
+    // replace the equivalent if the found decl has a higher access level.
+    auto existingDecl = Map.find(Key);
+
+    if ((existingDecl == Map.end()) ||
+        (Map[Key].first->getFormalAccess() < FD.first->getFormalAccess())) {
+      if (existingDecl != Map.end())
+        declsToReport.erase({existingDecl->getSecond().first});
+      Map[Key] = FD;
+      declsToReport.insert(FD);
     }
-    auto declName = VD->getFullName();
-    if ((foundVars.find(declName) == foundVars.end()) ||
-        (foundVars[declName].first->getFormalAccess() <
-         VD->getFormalAccess()))
-      foundVars[declName] = {VD, Reason};
   }
 
-  void feedFilteredResultsToParentConsumer() const {
-    for (auto entry: foundVars)
-      parentConsumer.foundDecl(entry.getSecond().first, entry.getSecond().second);
-    for (auto entry: foundFuncs)
-      parentConsumer.foundDecl(entry.getSecond().first, entry.getSecond().second);
-  }
-private:
-  CanType stripSelfRequirementsIfNeeded(ValueDecl *VD, GenericFunctionType *GFT) const {
+  CanType stripSelfRequirementsIfNeeded(ValueDecl *VD,
+                                        GenericFunctionType *GFT) const {
     // Preserve the generic signature if this is a subscript, which are uncurried,
     // or if we have generic params other than Self. Otherwise, use
     // the resultType of the curried function type.
@@ -555,6 +523,40 @@ class RestateFilteringConsumer : public VisibleDeclConsumer {
                                     GFT->getResult(), GFT->getExtInfo())
       ->getCanonicalType();
   }
+
+public:
+  RestateFilteringConsumer(Type baseTy, const DeclContext *DC,
+                           LazyResolver *resolver)
+  : resolver(resolver) {
+    assert(DC && baseTy && !baseTy->hasLValueType());
+  }
+
+  void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
+    assert(VD);
+    // If this isn't a protocol context, don't look further into the decl.
+    if (!isa<ProtocolDecl>(VD->getDeclContext())) {
+      declsToReport.insert({VD, Reason});
+      return;
+    }
+    if (resolver)
+      resolver->resolveDeclSignature(VD);
+
+    if (!VD->hasInterfaceType()) {
+      declsToReport.insert({VD, Reason});
+      return;
+    }
+    if (auto GFT = VD->getInterfaceType()->getAs<GenericFunctionType>()) {
+      auto type = stripSelfRequirementsIfNeeded(VD, GFT);
+      addDecl(foundFuncs, {VD->getFullName(), type}, {VD, Reason});
+      return;
+    }
+    addDecl(foundVars, VD->getFullName(), {VD, Reason});
+  }
+
+  void feedResultsToConsumer(VisibleDeclConsumer &Consumer) const {
+    for (const auto entry: declsToReport)
+      Consumer.foundDecl(entry.first, entry.second);
+  }
 };
 
 static void
@@ -933,14 +935,13 @@ static void lookupVisibleMemberDecls(
     LookupState LS, DeclVisibilityKind Reason, LazyResolver *TypeResolver,
     GenericSignatureBuilder *GSB) {
   OverrideFilteringConsumer overrideConsumer(BaseTy, CurrDC, TypeResolver);
-  RestateFilteringConsumer restateConsumer(overrideConsumer, BaseTy, CurrDC,
-                                           TypeResolver);
+  RestateFilteringConsumer restateConsumer(BaseTy, CurrDC, TypeResolver);
   VisitedSet Visited;
   lookupVisibleMemberDeclsImpl(BaseTy, restateConsumer, CurrDC, LS, Reason,
                                TypeResolver, GSB, Visited);
 
   // Report the declarations we found to the real consumer.
-  restateConsumer.feedFilteredResultsToParentConsumer();
+  restateConsumer.feedResultsToConsumer(overrideConsumer);
   for (const auto &DeclAndReason : overrideConsumer.DeclsToReport)
     Consumer.foundDecl(DeclAndReason.D, DeclAndReason.Reason);
 }

test/IDE/complete_associated_types.swift

@@ -267,7 +267,6 @@ struct StructWithBrokenConformance : FooProtocolWithAssociatedTypes {
 func testBrokenConformances1() {
   StructWithBrokenConformance.#^BROKEN_CONFORMANCE_1^#
 }
-
 // BROKEN_CONFORMANCE_1: Begin completions, 35 items
 // BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal:        DefaultedTypeCommonA[#StructWithBrokenConformance.DefaultedTypeCommonA#]; name=DefaultedTypeCommonA
 // BROKEN_CONFORMANCE_1-DAG: Decl[TypeAlias]/CurrNominal:        DefaultedTypeCommonB[#StructWithBrokenConformance.DefaultedTypeCommonB#]; name=DefaultedTypeCommonB

test/IDE/complete_pound_keypath.swift

@@ -40,6 +40,7 @@ func completeInKeyPath2() {
 
 // CHECK-IN_KEYPATH: Decl[InstanceVar]/CurrNominal:      prop1[#String#]; name=prop1
 // CHECK-IN_KEYPATH: Decl[InstanceVar]/CurrNominal:      prop2[#ObjCClass?#]; name=prop2
-// CHECK-IN_KEYPATH: Decl[InstanceVar]/Super:            hash[#Int#]; name=hash
 // CHECK-IN_KEYPATH: Decl[InstanceVar]/Super:            hashValue[#Int#]; name=hashValue
+// CHECK-IN_KEYPATH: Decl[InstanceVar]/Super:            hash[#Int#]; name=hash
+