From: Richard Smith Date: Wed, 29 Jul 2015 23:38:25 +0000 (+0000) Subject: [modules] When performing redeclaration lookup for a using declaration, prefer X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=98624f60f673d60493908cf2055d961be6675670;p=clang [modules] When performing redeclaration lookup for a using declaration, prefer UsingShadowDecls over other declarations of the same entity in the lookup results. This ensures that we build correct redeclaration chains for the UsingShadowDecls (otherwise we could see assertions and other misbehavior in modules builds, when merging combines multiple redeclaration chains for the same entity from the same module into one chain). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@243592 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index ac09b3c085..0f0c86f811 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -354,13 +354,45 @@ static DeclContext *getContextForScopeMatching(Decl *D) { return D->getDeclContext()->getRedeclContext(); } +/// \brief Determine whether \p D is a better lookup result than \p Existing, +/// given that they declare the same entity. +static bool isPreferredLookupResult(Sema::LookupNameKind Kind, + NamedDecl *D, NamedDecl *Existing) { + // When looking up redeclarations of a using declaration, prefer a using + // shadow declaration over any other declaration of the same entity. + if (Kind == Sema::LookupUsingDeclName && isa(D) && + !isa(Existing)) + return true; + + auto *DUnderlying = D->getUnderlyingDecl(); + auto *EUnderlying = Existing->getUnderlyingDecl(); + + // If they have different underlying declarations, pick one arbitrarily + // (this happens when two type declarations denote the same type). + // FIXME: Should we prefer a struct declaration over a typedef or vice versa? + // If a name could be a typedef-name or a class-name, which is it? + if (DUnderlying->getCanonicalDecl() != EUnderlying->getCanonicalDecl()) { + assert(isa(DUnderlying) && isa(EUnderlying)); + return false; + } + + // If D is newer than Existing, prefer it. + for (Decl *Prev = DUnderlying->getPreviousDecl(); Prev; + Prev = Prev->getPreviousDecl()) + if (Prev == EUnderlying) + return true; + + return false; +} + /// Resolves the result kind of this lookup. void LookupResult::resolveKind() { unsigned N = Decls.size(); // Fast case: no possible ambiguity. if (N == 0) { - assert(ResultKind == NotFound || ResultKind == NotFoundInCurrentInstantiation); + assert(ResultKind == NotFound || + ResultKind == NotFoundInCurrentInstantiation); return; } @@ -379,7 +411,7 @@ void LookupResult::resolveKind() { if (ResultKind == Ambiguous) return; llvm::SmallDenseMap Unique; - llvm::SmallPtrSet UniqueTypes; + llvm::SmallDenseMap UniqueTypes; bool Ambiguous = false; bool HasTag = false, HasFunction = false, HasNonFunction = false; @@ -398,42 +430,42 @@ void LookupResult::resolveKind() { continue; } + llvm::Optional ExistingI; + // Redeclarations of types via typedef can occur both within a scope // and, through using declarations and directives, across scopes. There is // no ambiguity if they all refer to the same type, so unique based on the // canonical type. if (TypeDecl *TD = dyn_cast(D)) { + // FIXME: Why are nested type declarations treated differently? if (!TD->getDeclContext()->isRecord()) { QualType T = getSema().Context.getTypeDeclType(TD); - if (!UniqueTypes.insert(getSema().Context.getCanonicalType(T)).second) { - // The type is not unique; pull something off the back and continue - // at this index. - Decls[I] = Decls[--N]; - continue; + auto UniqueResult = UniqueTypes.insert( + std::make_pair(getSema().Context.getCanonicalType(T), I)); + if (!UniqueResult.second) { + // The type is not unique. + ExistingI = UniqueResult.first->second; } } } - auto UniqueResult = Unique.insert(std::make_pair(D, I)); - if (!UniqueResult.second) { - // If it's not unique, pull something off the back (and - // continue at this index). - auto ExistingI = UniqueResult.first->second; - auto *Existing = Decls[ExistingI]->getUnderlyingDecl(); - for (Decl *Prev = Decls[I]->getUnderlyingDecl()->getPreviousDecl(); /**/; - Prev = Prev->getPreviousDecl()) { - if (Prev == Existing) { - // Existing result is older. Replace it with the new one. - Decls[ExistingI] = Decls[I]; - Decls[I] = Decls[--N]; - break; - } - if (!Prev) { - // New decl is older. Keep the existing one. - Decls[I] = Decls[--N]; - break; - } + // For non-type declarations, check for a prior lookup result naming this + // canonical declaration. + if (!ExistingI) { + auto UniqueResult = Unique.insert(std::make_pair(D, I)); + if (!UniqueResult.second) { + // We've seen this entity before. + ExistingI = UniqueResult.first->second; } + } + + if (ExistingI) { + // This is not a unique lookup result. Pick one of the results and + // discard the other. + if (isPreferredLookupResult(getLookupKind(), Decls[I], + Decls[*ExistingI])) + Decls[*ExistingI] = Decls[I]; + Decls[I] = Decls[--N]; continue; } diff --git a/test/Modules/Inputs/submodules-merge-defs/defs.h b/test/Modules/Inputs/submodules-merge-defs/defs.h index 07dfac7ee6..e4c86d80ba 100644 --- a/test/Modules/Inputs/submodules-merge-defs/defs.h +++ b/test/Modules/Inputs/submodules-merge-defs/defs.h @@ -97,3 +97,9 @@ namespace MergeFunctionTemplateSpecializations { enum ScopedEnum : int; enum ScopedEnum : int { a, b, c }; + +namespace RedeclDifferentDeclKind { + struct X {}; + typedef X X; + using RedeclDifferentDeclKind::X; +} diff --git a/test/Modules/Inputs/using-decl-redecl/a.h b/test/Modules/Inputs/using-decl-redecl/a.h new file mode 100644 index 0000000000..477546945c --- /dev/null +++ b/test/Modules/Inputs/using-decl-redecl/a.h @@ -0,0 +1,2 @@ +struct string {}; +namespace N { typedef ::string clstring; } diff --git a/test/Modules/Inputs/using-decl-redecl/b.h b/test/Modules/Inputs/using-decl-redecl/b.h new file mode 100644 index 0000000000..0714bb9914 --- /dev/null +++ b/test/Modules/Inputs/using-decl-redecl/b.h @@ -0,0 +1,3 @@ +#include "a.h" +namespace N { using ::N::clstring; } +extern N::clstring b; diff --git a/test/Modules/Inputs/using-decl-redecl/c.h b/test/Modules/Inputs/using-decl-redecl/c.h new file mode 100644 index 0000000000..e44e1a04e0 --- /dev/null +++ b/test/Modules/Inputs/using-decl-redecl/c.h @@ -0,0 +1,2 @@ +#include "b.h" +namespace N { using ::N::clstring; } diff --git a/test/Modules/Inputs/using-decl-redecl/module.modulemap b/test/Modules/Inputs/using-decl-redecl/module.modulemap new file mode 100644 index 0000000000..bd6ea830c2 --- /dev/null +++ b/test/Modules/Inputs/using-decl-redecl/module.modulemap @@ -0,0 +1,3 @@ +module a { header "a.h" } +module b { header "b.h" export * } +module c { header "c.h" export * } diff --git a/test/Modules/using-decl-redecl.cpp b/test/Modules/using-decl-redecl.cpp new file mode 100644 index 0000000000..0e78cec118 --- /dev/null +++ b/test/Modules/using-decl-redecl.cpp @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/using-decl-redecl/module.modulemap \ +// RUN: -I%S/Inputs/using-decl-redecl \ +// RUN: -verify %s +#include "c.h" +N::clstring y = b; + +// Use a typo to trigger import of all declarations in N. +N::clstrinh s; // expected-error {{did you mean 'clstring'}} +// expected-note@a.h:2 {{here}}