Author: rsmith Date: Wed Jul 29 18:38:25 2015 New Revision: 243592 URL: http://llvm.org/viewvc/llvm-project?rev=243592&view=rev Log: [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).
Added: cfe/trunk/test/Modules/Inputs/using-decl-redecl/ cfe/trunk/test/Modules/Inputs/using-decl-redecl/a.h cfe/trunk/test/Modules/Inputs/using-decl-redecl/b.h cfe/trunk/test/Modules/Inputs/using-decl-redecl/c.h cfe/trunk/test/Modules/Inputs/using-decl-redecl/module.modulemap cfe/trunk/test/Modules/using-decl-redecl.cpp Modified: cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=243592&r1=243591&r2=243592&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jul 29 18:38:25 2015 @@ -354,13 +354,45 @@ static DeclContext *getContextForScopeMa 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<UsingShadowDecl>(D) && + !isa<UsingShadowDecl>(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<TypeDecl>(DUnderlying) && isa<TypeDecl>(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<NamedDecl*, unsigned, 16> Unique; - llvm::SmallPtrSet<QualType, 16> UniqueTypes; + llvm::SmallDenseMap<QualType, unsigned, 16> UniqueTypes; bool Ambiguous = false; bool HasTag = false, HasFunction = false, HasNonFunction = false; @@ -398,42 +430,42 @@ void LookupResult::resolveKind() { continue; } + llvm::Optional<unsigned> 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<TypeDecl>(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; } Modified: cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h?rev=243592&r1=243591&r2=243592&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h (original) +++ cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h Wed Jul 29 18:38:25 2015 @@ -97,3 +97,9 @@ namespace MergeFunctionTemplateSpecializ enum ScopedEnum : int; enum ScopedEnum : int { a, b, c }; + +namespace RedeclDifferentDeclKind { + struct X {}; + typedef X X; + using RedeclDifferentDeclKind::X; +} Added: cfe/trunk/test/Modules/Inputs/using-decl-redecl/a.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/using-decl-redecl/a.h?rev=243592&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/using-decl-redecl/a.h (added) +++ cfe/trunk/test/Modules/Inputs/using-decl-redecl/a.h Wed Jul 29 18:38:25 2015 @@ -0,0 +1,2 @@ +struct string {}; +namespace N { typedef ::string clstring; } Added: cfe/trunk/test/Modules/Inputs/using-decl-redecl/b.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/using-decl-redecl/b.h?rev=243592&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/using-decl-redecl/b.h (added) +++ cfe/trunk/test/Modules/Inputs/using-decl-redecl/b.h Wed Jul 29 18:38:25 2015 @@ -0,0 +1,3 @@ +#include "a.h" +namespace N { using ::N::clstring; } +extern N::clstring b; Added: cfe/trunk/test/Modules/Inputs/using-decl-redecl/c.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/using-decl-redecl/c.h?rev=243592&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/using-decl-redecl/c.h (added) +++ cfe/trunk/test/Modules/Inputs/using-decl-redecl/c.h Wed Jul 29 18:38:25 2015 @@ -0,0 +1,2 @@ +#include "b.h" +namespace N { using ::N::clstring; } Added: cfe/trunk/test/Modules/Inputs/using-decl-redecl/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/using-decl-redecl/module.modulemap?rev=243592&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/using-decl-redecl/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/using-decl-redecl/module.modulemap Wed Jul 29 18:38:25 2015 @@ -0,0 +1,3 @@ +module a { header "a.h" } +module b { header "b.h" export * } +module c { header "c.h" export * } Added: cfe/trunk/test/Modules/using-decl-redecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-decl-redecl.cpp?rev=243592&view=auto ============================================================================== --- cfe/trunk/test/Modules/using-decl-redecl.cpp (added) +++ cfe/trunk/test/Modules/using-decl-redecl.cpp Wed Jul 29 18:38:25 2015 @@ -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}} _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits