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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits