Merge pull request #26825 from slavapestov/new-shadowing-rule-part-0

slavapestov · web-flow · commit b111ea4a0fb3 · 2019-08-26T13:43:58.000-04:00
Few more small name lookup cleanups
diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
@@ -868,15 +868,14 @@ StructDecl *ASTContext::getObjCBoolDecl() const {
 ClassDecl *ASTContext::get##NAME##Decl() const { \
   if (!getImpl().NAME##Decl) { \
     if (ModuleDecl *M = getLoadedModule(Id_Foundation)) { \
-      /* Note: use unqualified lookup so we find NSError regardless of */ \
-      /* whether it's defined in the Foundation module or the Clang */ \
-      /* Foundation module it imports. */ \
-      UnqualifiedLookup lookup(getIdentifier(#NAME), M); \
-      if (auto type = lookup.getSingleTypeResult()) { \
-        if (auto classDecl = dyn_cast<ClassDecl>(type)) { \
-          if (classDecl->getGenericParams() == nullptr) { \
-            getImpl().NAME##Decl = classDecl; \
-          } \
+      /* Note: lookupQualified() will search both the Foundation module \
+       * and the Clang Foundation module it imports. */ \
+      SmallVector<ValueDecl *, 1> decls; \
+      M->lookupQualified(M, getIdentifier(#NAME), NL_OnlyTypes, decls); \
+      if (decls.size() == 1 && isa<ClassDecl>(decls[0])) { \
+        auto classDecl = cast<ClassDecl>(decls[0]); \
+        if (classDecl->getGenericParams() == nullptr) { \
+          getImpl().NAME##Decl = classDecl; \
         } \
       } \
     } \
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -1613,14 +1613,16 @@ bool DeclContext::lookupQualified(ModuleDecl *module, DeclName member,
   bool isLookupCascading;
   configureLookup(this, options, tracker, isLookupCascading);
 
+  auto kind = (options & NL_OnlyTypes
+               ? ResolutionKind::TypesOnly
+               : ResolutionKind::Overloadable);
   auto topLevelScope = getModuleScopeContext();
   if (module == topLevelScope->getParentModule()) {
     if (tracker) {
       recordLookupOfTopLevelName(topLevelScope, member, isLookupCascading);
     }
     lookupInModule(module, /*accessPath=*/{}, member, decls,
-                   NLKind::QualifiedLookup, ResolutionKind::Overloadable,
-                   topLevelScope);
+                   NLKind::QualifiedLookup, kind, topLevelScope);
   } else {
     // Note: This is a lookup into another module. Unless we're compiling
     // multiple modules at once, or if the other module re-exports this one,
@@ -1633,8 +1635,7 @@ bool DeclContext::lookupQualified(ModuleDecl *module, DeclName member,
       if (import.second != module)
         return true;
       lookupInModule(import.second, import.first, member, decls,
-                     NLKind::QualifiedLookup, ResolutionKind::Overloadable,
-                     topLevelScope);
+                     NLKind::QualifiedLookup, kind, topLevelScope);
       // If we're able to do an unscoped lookup, we see everything. No need
       // to keep going.
       return !import.first.empty();
@@ -1644,10 +1645,6 @@ bool DeclContext::lookupQualified(ModuleDecl *module, DeclName member,
   llvm::SmallPtrSet<ValueDecl *, 4> knownDecls;
   decls.erase(std::remove_if(decls.begin(), decls.end(),
                              [&](ValueDecl *vd) -> bool {
-    // If we're performing a type lookup, skip non-types.
-    if ((options & NL_OnlyTypes) && !isa<TypeDecl>(vd))
-      return true;
-
     return !knownDecls.insert(vd).second;
   }), decls.end());
 
diff --git a/lib/ParseSIL/ParseSIL.cpp b/lib/ParseSIL/ParseSIL.cpp
@@ -1170,14 +1170,19 @@ bool SILParser::performTypeLocChecking(TypeLoc &T, bool IsSILType,
 
 /// Find the top-level ValueDecl or Module given a name.
 static llvm::PointerUnion<ValueDecl *, ModuleDecl *>
-lookupTopDecl(Parser &P, DeclBaseName Name) {
+lookupTopDecl(Parser &P, DeclBaseName Name, bool typeLookup) {
   // Use UnqualifiedLookup to look through all of the imports.
   // We have to lie and say we're done with parsing to make this happen.
   assert(P.SF.ASTStage == SourceFile::Parsing &&
          "Unexpected stage during parsing!");
   llvm::SaveAndRestore<SourceFile::ASTStage_t> ASTStage(P.SF.ASTStage,
                                                         SourceFile::Parsed);
-  UnqualifiedLookup DeclLookup(Name, &P.SF);
+
+  UnqualifiedLookup::Options options;
+  if (typeLookup)
+    options |= UnqualifiedLookup::Flags::TypeLookup;
+
+  UnqualifiedLookup DeclLookup(Name, &P.SF, SourceLoc(), options);
   assert(DeclLookup.isSuccess() && DeclLookup.Results.size() == 1);
   ValueDecl *VD = DeclLookup.Results.back().getValueDecl();
   return VD;
@@ -1358,9 +1363,11 @@ bool SILParser::parseSILDottedPathWithoutPound(ValueDecl *&Decl,
     }
   } while (P.consumeIf(tok::period));
 
-  // Look up ValueDecl from a dotted path.
+  // Look up ValueDecl from a dotted path. If there are multiple components,
+  // the first one must be a type declaration.
   ValueDecl *VD;
-  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res = lookupTopDecl(P, FullName[0]);
+  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res = lookupTopDecl(
+    P, FullName[0], /*typeLookup=*/FullName.size() > 1);
   // It is possible that the last member lookup can return multiple lookup
   // results. One example is the overloaded member functions.
   if (Res.is<ModuleDecl*>()) {
@@ -5749,7 +5756,8 @@ bool SILParserTUState::parseSILVTable(Parser &P) {
     return true;
 
   // Find the class decl.
-  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res = lookupTopDecl(P, Name);
+  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res =
+    lookupTopDecl(P, Name, /*typeLookup=*/true);
   assert(Res.is<ValueDecl*>() && "Class look-up should return a Decl");
   ValueDecl *VD = Res.get<ValueDecl*>();
   if (!VD) {
@@ -5836,7 +5844,8 @@ static ProtocolDecl *parseProtocolDecl(Parser &P, SILParser &SP) {
     return nullptr;
 
   // Find the protocol decl. The protocol can be imported.
-  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res = lookupTopDecl(P, DeclName);
+  llvm::PointerUnion<ValueDecl*, ModuleDecl *> Res =
+    lookupTopDecl(P, DeclName, /*typeLookup=*/true);
   assert(Res.is<ValueDecl*>() && "Protocol look-up should return a Decl");
   ValueDecl *VD = Res.get<ValueDecl*>();
   if (!VD) {
diff --git a/lib/PrintAsObjC/PrintAsObjC.cpp b/lib/PrintAsObjC/PrintAsObjC.cpp
@@ -950,11 +950,14 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
       if (!renamedParsedDeclName.ContextName.empty()) {
         return nullptr;
       }
-      UnqualifiedLookup lookup(renamedDeclName.getBaseIdentifier(),
-                               declContext->getModuleScopeContext(),
-                               SourceLoc(),
-                               UnqualifiedLookup::Flags::TypeLookup);
-      return lookup.getSingleTypeResult();
+      SmallVector<ValueDecl *, 1> decls;
+      declContext->lookupQualified(declContext->getParentModule(),
+                                   renamedDeclName.getBaseIdentifier(),
+                                   NL_OnlyTypes,
+                                   decls);
+      if (decls.size() == 1)
+        return decls[0];
+      return nullptr;
     }
 
     TypeDecl *typeDecl = declContext->getSelfNominalTypeDecl();
@@ -1439,10 +1442,12 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
              isNSObjectOrAnyHashable(ctx, typeArgs[0])) {
       if (ModuleDecl *M = ctx.getLoadedModule(ctx.Id_Foundation)) {
         if (!NSCopyingType) {
-          UnqualifiedLookup lookup(ctx.getIdentifier("NSCopying"), M);
-          auto type = lookup.getSingleTypeResult();
-          if (type && isa<ProtocolDecl>(type)) {
-            NSCopyingType = type->getDeclaredInterfaceType();
+          SmallVector<ValueDecl *, 1> decls;
+          M->lookupQualified(M, ctx.getIdentifier("NSCopying"),
+                             NL_OnlyTypes, decls);
+          if (decls.size() == 1 && isa<ProtocolDecl>(decls[0])) {
+            NSCopyingType = cast<ProtocolDecl>(decls[0])
+              ->getDeclaredInterfaceType();
           } else {
             NSCopyingType = Type();
           }
diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp
@@ -74,7 +74,6 @@ namespace {
     LookupResult &Result;
     DeclContext *DC;
     NameLookupOptions Options;
-    bool IsMemberLookup;
 
     /// The vector of found declarations.
     SmallVector<ValueDecl *, 4> FoundDecls;
@@ -86,46 +85,13 @@ namespace {
 
   public:
     LookupResultBuilder(LookupResult &result, DeclContext *dc,
-                        NameLookupOptions options,
-                        bool isMemberLookup)
-      : Result(result), DC(dc), Options(options),
-        IsMemberLookup(isMemberLookup) {
+                        NameLookupOptions options)
+      : Result(result), DC(dc), Options(options) {
       if (dc->getASTContext().isAccessControlDisabled())
         Options |= NameLookupFlags::IgnoreAccessControl;
     }
 
-    /// Determine whether we should filter out the results by removing
-    /// overridden and shadowed declarations.
-    /// FIXME: We should *always* do this, but there are weird assumptions
-    /// about the results of unqualified name lookup, e.g., that a local
-    /// variable not having a type indicates that it hasn't been seen yet.
-    bool shouldFilterResults() const {
-      // Member lookups always filter results.
-      if (IsMemberLookup) return true;
-
-      bool allAreInOtherModules = true;
-      auto currentModule = DC->getParentModule();
-      for (const auto &found : Result) {
-        // We found a member, so we need to filter.
-        if (found.getBaseDecl() != nullptr)
-          return true;
-
-        // We found something in our own module.
-        if (found.getValueDecl()->getDeclContext()->getParentModule() ==
-              currentModule)
-          allAreInOtherModules = false;
-      }
-
-      // FIXME: Only perform shadowing if we found things from other modules.
-      // This prevents us from introducing additional type-checking work
-      // during name lookup.
-      return allAreInOtherModules;
-    }
-
     ~LookupResultBuilder() {
-      // Check whether we should do this filtering aat all.
-      if (!shouldFilterResults()) return;
-
       // Remove any overridden declarations from the found-declarations set.
       removeOverriddenDecls(FoundDecls);
       removeOverriddenDecls(FoundOuterDecls);
@@ -305,7 +271,7 @@ LookupResult TypeChecker::lookupUnqualified(DeclContext *dc, DeclName name,
                            convertToUnqualifiedLookupOptions(options));
 
   LookupResult result;
-  LookupResultBuilder builder(result, dc, options, /*memberLookup*/false);
+  LookupResultBuilder builder(result, dc, options);
   for (auto idx : indices(lookup.Results)) {
     const auto &found = lookup.Results[idx];
     // Determine which type we looked through to find this result.
@@ -381,8 +347,7 @@ LookupResult TypeChecker::lookupMember(DeclContext *dc,
   subOptions &= ~NL_RemoveOverridden;
   subOptions &= ~NL_RemoveNonVisible;
 
-  LookupResultBuilder builder(result, dc, options,
-                              /*memberLookup*/true);
+  LookupResultBuilder builder(result, dc, options);
   SmallVector<ValueDecl *, 4> lookupResults;
   dc->lookupQualified(type, name, subOptions, lookupResults);
 
