Merge pull request #18562 from DougGregor/remove-shadowed-without-validation

[Name lookup] Make shadowing less reliant on the type checker.

Author: DougGregor

lib/AST/NameLookup.cpp

@@ -157,27 +157,165 @@ static ConstructorComparison compareConstructors(ConstructorDecl *ctor1,
   return ConstructorComparison::Same;
 }
 
-bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
-                                const ModuleDecl *curModule) {
-  auto typeResolver = curModule->getASTContext().getLazyResolver();
-
-  // Category declarations by their signatures.
-  llvm::SmallDenseMap<std::pair<CanType, DeclBaseName>,
-                      llvm::TinyPtrVector<ValueDecl *>>
-    CollidingDeclGroups;
-
-  /// Objective-C initializers are tracked by their context type and
-  /// full name.
-  llvm::SmallDenseMap<std::pair<CanType, DeclName>, 
-                      llvm::TinyPtrVector<ConstructorDecl *>>
-    ObjCCollidingConstructors;
-  bool anyCollisions = false;
+/// Given a set of declarations whose names and signatures have matched,
+/// figure out which of these declarations have been shadowed by others.
+static void recordShadowedDeclsAfterSignatureMatch(
+                              ArrayRef<ValueDecl *> decls,
+                              const ModuleDecl *curModule,
+                              llvm::SmallPtrSetImpl<ValueDecl *> &shadowed) {
+  assert(decls.size() > 1 && "Nothing collided");
+
+  // Compare each declaration to every other declaration. This is
+  // unavoidably O(n^2) in the number of declarations, but because they
+  // all have the same signature, we expect n to remain small.
+  ASTContext &ctx = curModule->getASTContext();
+  for (unsigned firstIdx : indices(decls)) {
+    auto firstDecl = decls[firstIdx];
+    auto firstModule = firstDecl->getModuleContext();
+    for (unsigned secondIdx : range(firstIdx + 1, decls.size())) {
+      // Determine whether one module takes precedence over another.
+      auto secondDecl = decls[secondIdx];
+      auto secondModule = secondDecl->getModuleContext();
+
+      // If one declaration is in a protocol or extension thereof and the
+      // other is not, prefer the one that is not.
+      if ((bool)firstDecl->getDeclContext()
+            ->getAsProtocolOrProtocolExtensionContext()
+            != (bool)secondDecl->getDeclContext()
+                 ->getAsProtocolOrProtocolExtensionContext()) {
+        if (firstDecl->getDeclContext()
+              ->getAsProtocolOrProtocolExtensionContext()) {
+          shadowed.insert(firstDecl);
+          break;
+        } else {
+          shadowed.insert(secondDecl);
+          continue;
+        }
+      }
+
+      // If one declaration is available and the other is not, prefer the
+      // available one.
+      if (firstDecl->getAttrs().isUnavailable(ctx) !=
+            secondDecl->getAttrs().isUnavailable(ctx)) {
+       if (firstDecl->getAttrs().isUnavailable(ctx)) {
+         shadowed.insert(firstDecl);
+         break;
+       } else {
+         shadowed.insert(secondDecl);
+         continue;
+       }
+      }
+
+      // Don't apply module-shadowing rules to members of protocol types.
+      if (isa<ProtocolDecl>(firstDecl->getDeclContext()) ||
+          isa<ProtocolDecl>(secondDecl->getDeclContext()))
+        continue;
+
+      // Prefer declarations in the current module over those in another
+      // module.
+      // FIXME: This is a hack. We should query a (lazily-built, cached)
+      // module graph to determine shadowing.
+      if ((firstModule == curModule) != (secondModule == curModule)) {
+        // If the first module is the current module, the second declaration
+        // is shadowed by the first.
+        if (firstModule == curModule) {
+          shadowed.insert(secondDecl);
+          continue;
+        }
+
+        // Otherwise, the first declaration is shadowed by the second. There is
+        // no point in continuing to compare the first declaration to others.
+        shadowed.insert(firstDecl);
+        break;
+      }
+
+      // Prefer declarations in an overlay to similar declarations in
+      // the Clang module it customizes.
+      if (firstDecl->hasClangNode() != secondDecl->hasClangNode()) {
+        auto clangLoader = ctx.getClangModuleLoader();
+        if (!clangLoader) continue;
+
+        if (clangLoader->isInOverlayModuleForImportedModule(
+                                              firstDecl->getDeclContext(),
+                                              secondDecl->getDeclContext())) {
+          shadowed.insert(secondDecl);
+          continue;
+        }
+
+        if (clangLoader->isInOverlayModuleForImportedModule(
+                                               secondDecl->getDeclContext(),
+                                               firstDecl->getDeclContext())) {
+          shadowed.insert(firstDecl);
+          break;
+        }
+      }
+    }
+  }
+}
+
+/// Look through the given set of declarations (that all have the same name),
+/// recording those that are shadowed by another declaration in the
+/// \c shadowed set.
+static void recordShadowDeclsAfterObjCInitMatch(
+                                ArrayRef<ConstructorDecl *> ctors,
+                                llvm::SmallPtrSetImpl<ValueDecl *> &shadowed) {
+  assert(ctors.size() > 1 && "No collisions");
+
+  ASTContext &ctx = ctors.front()->getASTContext();
+
+  // Find the "best" constructor with this signature.
+  ConstructorDecl *bestCtor = ctors[0];
+  for (auto ctor : ctors.slice(1)) {
+    auto comparison = compareConstructors(ctor, bestCtor, ctx);
+    if (comparison == ConstructorComparison::Better)
+      bestCtor = ctor;
+  }
+
+  // Shadow any initializers that are worse.
+  for (auto ctor : ctors) {
+    auto comparison = compareConstructors(ctor, bestCtor, ctx);
+    if (comparison == ConstructorComparison::Worse)
+      shadowed.insert(ctor);
+  }
+}
+
+/// Look through the given set of declarations (that all have the same name),
+/// recording those that are shadowed by another declaration in the
+/// \c shadowed set.
+static void recordShadowedDecls(ArrayRef<ValueDecl *> decls,
+                                const ModuleDecl *curModule,
+                                llvm::SmallPtrSetImpl<ValueDecl *> &shadowed) {
+  if (decls.size() < 2)
+    return;
+
+  auto typeResolver = decls[0]->getASTContext().getLazyResolver();
+
+  // Categorize all of the declarations based on their overload signatures.
+  llvm::SmallDenseMap<CanType, llvm::TinyPtrVector<ValueDecl *>> collisions;
+  llvm::SmallVector<CanType, 2> collisionTypes;
+  llvm::SmallDenseMap<NominalTypeDecl *, llvm::TinyPtrVector<ConstructorDecl *>>
+    objCInitializerCollisions;
+  llvm::TinyPtrVector<NominalTypeDecl *> objCInitializerCollisionNominals;
+
   for (auto decl : decls) {
-    // FIXME: Egregious hack to avoid failing when there are no declared types.
-    // FIXME: Pass this down instead of getting it from the ASTContext.
+    // Specifically keep track of Objective-C initializers, which can come from
+    // either init methods or factory methods.
+    if (decl->hasClangNode()) {
+      if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
+        auto nominal = ctor->getDeclContext()
+            ->getAsNominalTypeOrNominalTypeExtensionContext();
+        auto &knownInits = objCInitializerCollisions[nominal];
+        if (knownInits.size() == 1) {
+          objCInitializerCollisionNominals.push_back(nominal);
+        }
+        knownInits.push_back(ctor);
+      }
+    }
+
+    // We need an interface type here.
     if (typeResolver)
       typeResolver->resolveDeclSignature(decl);
-    
+
     // If the decl is currently being validated, this is likely a recursive
     // reference and we'll want to skip ahead so as to avoid having its type
     // attempt to desugar itself.
@@ -195,154 +333,60 @@ bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
     if (auto asd = dyn_cast<AbstractStorageDecl>(decl))
       signature = asd->getOverloadSignatureType();
 
-    // If we've seen a declaration with this signature before, note it.
-    auto &knownDecls =
-        CollidingDeclGroups[std::make_pair(signature, decl->getBaseName())];
+    // Record this declaration based on its signature.
+    auto &known = collisions[signature];
+    if (known.size() == 1) {
+      collisionTypes.push_back(signature);
+    }
+    known.push_back(decl);
+  }
+
+  // Check whether we have shadowing for signature collisions.
+  for (auto signature : collisionTypes) {
+    recordShadowedDeclsAfterSignatureMatch(collisions[signature], curModule,
+                                           shadowed);
+  }
+
+  // Check whether we have shadowing for Objective-C initializer collisions.
+  for (auto nominal : objCInitializerCollisionNominals) {
+    recordShadowDeclsAfterObjCInitMatch(objCInitializerCollisions[nominal],
+                                        shadowed);
+  }
+}
+
+bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
+                                const ModuleDecl *curModule) {
+  // Collect declarations with the same (full) name.
+  llvm::SmallDenseMap<DeclName, llvm::TinyPtrVector<ValueDecl *>>
+    collidingDeclGroups;
+  bool anyCollisions = false;
+  for (auto decl : decls) {
+    // Record this declaration based on its full name.
+    auto &knownDecls = collidingDeclGroups[decl->getFullName()];
     if (!knownDecls.empty())
       anyCollisions = true;
 
     knownDecls.push_back(decl);
-
-    // Specifically keep track of Objective-C initializers, which can come from
-    // either init methods or factory methods.
-    if (decl->hasClangNode()) {
-      if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
-        auto ctorSignature
-          = std::make_pair(ctor->getDeclContext()->getDeclaredInterfaceType()
-                               ->getCanonicalType(),
-                           decl->getFullName());
-        auto &knownCtors = ObjCCollidingConstructors[ctorSignature];
-        if (!knownCtors.empty())
-          anyCollisions = true;
-        knownCtors.push_back(ctor);
-      }
-    }
   }
 
-  // If there were no signature collisions, there is nothing to do.
+  // If nothing collided, we're done.
   if (!anyCollisions)
     return false;
 
-  // Determine the set of declarations that are shadowed by other declarations.
+  // Walk through the declarations again, marking any declarations that shadow.
   llvm::SmallPtrSet<ValueDecl *, 4> shadowed;
-  ASTContext &ctx = decls[0]->getASTContext();
-  for (auto &collidingDecls : CollidingDeclGroups) {
-    // If only one declaration has this signature, it isn't shadowed by
-    // anything.
-    if (collidingDecls.second.size() == 1)
-      continue;
-
-    // Compare each declaration to every other declaration. This is
-    // unavoidably O(n^2) in the number of declarations, but because they
-    // all have the same signature, we expect n to remain small.
-    for (unsigned firstIdx = 0, n = collidingDecls.second.size();
-         firstIdx != n; ++firstIdx) {
-      auto firstDecl = collidingDecls.second[firstIdx];
-      auto firstModule = firstDecl->getModuleContext();
-      for (unsigned secondIdx = firstIdx + 1; secondIdx != n; ++secondIdx) {
-        // Determine whether one module takes precedence over another.
-        auto secondDecl = collidingDecls.second[secondIdx];
-        auto secondModule = secondDecl->getModuleContext();
-
-        // If one declaration is in a protocol or extension thereof and the
-        // other is not, prefer the one that is not.
-        if ((bool)firstDecl->getDeclContext()
-              ->getAsProtocolOrProtocolExtensionContext()
-              != (bool)secondDecl->getDeclContext()
-                   ->getAsProtocolOrProtocolExtensionContext()) {
-          if (firstDecl->getDeclContext()
-                ->getAsProtocolOrProtocolExtensionContext()) {
-            shadowed.insert(firstDecl);
-            break;
-          } else {
-            shadowed.insert(secondDecl);
-            continue;
-          }
-        }
-
-        // If one declaration is available and the other is not, prefer the
-        // available one.
-        if (firstDecl->getAttrs().isUnavailable(ctx) !=
-              secondDecl->getAttrs().isUnavailable(ctx)) {
-         if (firstDecl->getAttrs().isUnavailable(ctx)) {
-           shadowed.insert(firstDecl);
-           break;
-         } else {
-           shadowed.insert(secondDecl);
-           continue;
-         }
-        }
-
-        // Don't apply module-shadowing rules to members of protocol types.
-        if (isa<ProtocolDecl>(firstDecl->getDeclContext()) ||
-            isa<ProtocolDecl>(secondDecl->getDeclContext()))
-          continue;
-
-        // Prefer declarations in the current module over those in another
-        // module.
-        // FIXME: This is a hack. We should query a (lazily-built, cached)
-        // module graph to determine shadowing.
-        if ((firstModule == curModule) != (secondModule == curModule)) {
-          // If the first module is the current module, the second declaration
-          // is shadowed by the first.
-          if (firstModule == curModule) {
-            shadowed.insert(secondDecl);
-            continue;
-          }
-
-          // Otherwise, the first declaration is shadowed by the second. There is
-          // no point in continuing to compare the first declaration to others.
-          shadowed.insert(firstDecl);
-          break;
-        }
-
-        // Prefer declarations in an overlay to similar declarations in
-        // the Clang module it customizes.
-        if (firstDecl->hasClangNode() != secondDecl->hasClangNode()) {
-          auto clangLoader = ctx.getClangModuleLoader();
-          if (!clangLoader) continue;
-
-          if (clangLoader->isInOverlayModuleForImportedModule(
-                                                firstDecl->getDeclContext(),
-                                                secondDecl->getDeclContext())) {
-            shadowed.insert(secondDecl);
-            continue;
-          }
-
-          if (clangLoader->isInOverlayModuleForImportedModule(
-                                                 secondDecl->getDeclContext(),
-                                                 firstDecl->getDeclContext())) {
-            shadowed.insert(firstDecl);
-            break;
-          }
-        }
-      }
-    }
-  }
-  
-  // Check for collisions among Objective-C initializers. When such collisions
-  // exist, we pick the
-  for (const auto &colliding : ObjCCollidingConstructors) {
-    if (colliding.second.size() == 1)
+  for (auto decl : decls) {
+    auto known = collidingDeclGroups.find(decl->getFullName());
+    if (known == collidingDeclGroups.end()) {
+      // We already handled this group.
       continue;
-
-    // Find the "best" constructor with this signature.
-    ConstructorDecl *bestCtor = colliding.second[0];
-    for (auto ctor : colliding.second) {
-      auto comparison = compareConstructors(ctor, bestCtor, ctx);
-      if (comparison == ConstructorComparison::Better)
-        bestCtor = ctor;
     }
 
-    // Shadow any initializers that are worse.
-    for (auto ctor : colliding.second) {
-      auto comparison = compareConstructors(ctor, bestCtor, ctx);
-      if (comparison == ConstructorComparison::Worse)
-        shadowed.insert(ctor);
-    }
+    recordShadowedDecls(known->second, curModule, shadowed);
+    collidingDeclGroups.erase(known);
   }
 
-  // If none of the declarations were shadowed, we're done.
+  // If no declarations were shadowed, we're done.
   if (shadowed.empty())
     return false;
 
@@ -2277,8 +2321,7 @@ directReferencesForTypeRepr(Evaluator &evaluator,
   }
 }
 
-static DirectlyReferencedTypeDecls
-directReferencesForType(Type type) {
+static DirectlyReferencedTypeDecls directReferencesForType(Type type) {
   // If it's a typealias, return that.
   if (auto aliasType = dyn_cast<NameAliasType>(type.getPointer()))
     return { 1, aliasType->getDecl() };

lib/Sema/CSApply.cpp

@@ -8085,6 +8085,7 @@ Solution::convertBooleanTypeToBuiltinI1(Expr *expr,
   }
 
   // The method is not generic, so there are no substitutions.
+  tc.validateDeclForNameLookup(builtinMethod);
   auto builtinMethodType = builtinMethod->getInterfaceType()
     ->castTo<FunctionType>();