r243592 - [modules] When performing redeclaration lookup for a using declaration, prefer
Richard Smith
richard-llvm at metafoo.co.uk
Wed Jul 29 16:38:25 PDT 2015
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 at a.h:2 {{here}}
More information about the cfe-commits
mailing list