r321916 - When name lookup finds a non-imported declaration and looks back along the

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Jan  5 16:09:23 2018
New Revision: 321916

URL: http://llvm.org/viewvc/llvm-project?rev=321916&view=rev
Log:
When name lookup finds a non-imported declaration and looks back along the
redecl chain for an imported declaration, make sure to check the IDNS of prior
imported decls.

Otherwise we can end up finding an invisible friend declaration and incorrectly
believing that it should be visible.

Added:
    cfe/trunk/test/Modules/using-decl-friend.cpp
Modified:
    cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=321916&r1=321915&r2=321916&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Jan  5 16:09:23 2018
@@ -1656,7 +1656,8 @@ bool Sema::shouldLinkPossiblyHiddenDecl(
 ///
 /// \returns D, or a visible previous declaration of D, whichever is more recent
 /// and visible. If no declaration of D is visible, returns null.
-static NamedDecl *findAcceptableDecl(Sema &SemaRef, NamedDecl *D) {
+static NamedDecl *findAcceptableDecl(Sema &SemaRef, NamedDecl *D,
+                                     unsigned IDNS) {
   assert(!LookupResult::isVisible(SemaRef, D) && "not in slow case");
 
   for (auto RD : D->redecls()) {
@@ -1668,7 +1669,8 @@ static NamedDecl *findAcceptableDecl(Sem
     // FIXME: This is wrong in the case where the previous declaration is not
     // visible in the same scope as D. This needs to be done much more
     // carefully.
-    if (LookupResult::isVisible(SemaRef, ND))
+    if (ND->isInIdentifierNamespace(IDNS) &&
+        LookupResult::isVisible(SemaRef, ND))
       return ND;
   }
 
@@ -1693,14 +1695,15 @@ NamedDecl *LookupResult::getAcceptableDe
     auto *Key = ND->getCanonicalDecl();
     if (auto *Acceptable = getSema().VisibleNamespaceCache.lookup(Key))
       return Acceptable;
-    auto *Acceptable =
-        isVisible(getSema(), Key) ? Key : findAcceptableDecl(getSema(), Key);
+    auto *Acceptable = isVisible(getSema(), Key)
+                           ? Key
+                           : findAcceptableDecl(getSema(), Key, IDNS);
     if (Acceptable)
       getSema().VisibleNamespaceCache.insert(std::make_pair(Key, Acceptable));
     return Acceptable;
   }
 
-  return findAcceptableDecl(getSema(), D);
+  return findAcceptableDecl(getSema(), D, IDNS);
 }
 
 /// @brief Perform unqualified name lookup starting from a given
@@ -3329,6 +3332,23 @@ void Sema::ArgumentDependentLookup(Decla
     //        lookup (11.4).
     DeclContext::lookup_result R = NS->lookup(Name);
     for (auto *D : R) {
+      auto *Underlying = D;
+      if (auto *USD = dyn_cast<UsingShadowDecl>(D))
+        Underlying = USD->getTargetDecl();
+
+      if (!isa<FunctionDecl>(Underlying) &&
+          !isa<FunctionTemplateDecl>(Underlying))
+        continue;
+
+      if (!isVisible(D)) {
+        D = findAcceptableDecl(
+            *this, D, (Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend));
+        if (!D)
+          continue;
+        if (auto *USD = dyn_cast<UsingShadowDecl>(D))
+          Underlying = USD->getTargetDecl();
+      }
+
       // If the only declaration here is an ordinary friend, consider
       // it only if it was declared in an associated classes.
       if ((D->getIdentifierNamespace() & Decl::IDNS_Ordinary) == 0) {
@@ -3350,22 +3370,6 @@ void Sema::ArgumentDependentLookup(Decla
           continue;
       }
 
-      auto *Underlying = D;
-      if (auto *USD = dyn_cast<UsingShadowDecl>(D))
-        Underlying = USD->getTargetDecl();
-
-      if (!isa<FunctionDecl>(Underlying) &&
-          !isa<FunctionTemplateDecl>(Underlying))
-        continue;
-
-      if (!isVisible(D)) {
-        D = findAcceptableDecl(*this, D);
-        if (!D)
-          continue;
-        if (auto *USD = dyn_cast<UsingShadowDecl>(D))
-          Underlying = USD->getTargetDecl();
-      }
-
       // FIXME: Preserve D as the FoundDecl.
       Result.insert(Underlying);
     }
@@ -3865,17 +3869,13 @@ static void checkCorrectionVisibility(Se
   bool AnyVisibleDecls = !NewDecls.empty();
 
   for (/**/; DI != DE; ++DI) {
-    NamedDecl *VisibleDecl = *DI;
-    if (!LookupResult::isVisible(SemaRef, *DI))
-      VisibleDecl = findAcceptableDecl(SemaRef, *DI);
-
-    if (VisibleDecl) {
+    if (LookupResult::isVisible(SemaRef, *DI)) {
       if (!AnyVisibleDecls) {
         // Found a visible decl, discard all hidden ones.
         AnyVisibleDecls = true;
         NewDecls.clear();
       }
-      NewDecls.push_back(VisibleDecl);
+      NewDecls.push_back(*DI);
     } else if (!AnyVisibleDecls && !(*DI)->isModulePrivate())
       NewDecls.push_back(*DI);
   }
@@ -3945,8 +3945,7 @@ void TypoCorrectionConsumer::FoundDecl(N
 
   // Only consider visible declarations and declarations from modules with
   // names that exactly match.
-  if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo &&
-      !findAcceptableDecl(SemaRef, ND))
+  if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo)
     return;
 
   FoundName(Name->getName());

Added: cfe/trunk/test/Modules/using-decl-friend.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-decl-friend.cpp?rev=321916&view=auto
==============================================================================
--- cfe/trunk/test/Modules/using-decl-friend.cpp (added)
+++ cfe/trunk/test/Modules/using-decl-friend.cpp Fri Jan  5 16:09:23 2018
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fmodules %s -verify
+// expected-no-diagnostics
+
+#pragma clang module build A
+module A {}
+#pragma clang module contents
+#pragma clang module begin A
+namespace N {
+  class X;
+}
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module build B
+module B {
+  module X {}
+  module Y {}
+}
+#pragma clang module contents
+#pragma clang module begin B.X
+namespace N {
+  class Friendly {
+    friend class X;
+  };
+}
+#pragma clang module end
+#pragma clang module begin B.Y
+namespace N {
+  class X;
+}
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module import A
+#pragma clang module import B.X
+using N::X;
+X *p;