From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Mon, 24 Jun 2013 01:46:41 +0000 (+0000)
Subject: When setting the external visible declarations for a decl context, check
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ddb10f767604d8efa5e491076d6fdd23a19db86c;p=clang

When setting the external visible declarations for a decl context, check
whether they replace any existing lookups in the context, rather than
accumulating a bunch of lookup results referring to the same entity.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@184679 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 084a4321d8..fb13766ad0 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -1012,13 +1012,38 @@ ExternalASTSource::SetExternalVisibleDeclsForName(const DeclContext *DC,
     Map = DC->CreateStoredDeclsMap(Context);
 
   StoredDeclsList &List = (*Map)[Name];
-  for (ArrayRef<NamedDecl*>::iterator
-         I = Decls.begin(), E = Decls.end(); I != E; ++I) {
-    if (List.isNull())
-      List.setOnlyValue(*I);
-    else
-      // FIXME: Need declarationReplaces handling for redeclarations in modules.
-      List.AddSubsequentDecl(*I);
+
+  // Clear out any old external visible declarations, to avoid quadratic
+  // performance in the redeclaration checks below.
+  List.removeExternalDecls();
+
+  if (!List.isNull()) {
+    // We have both existing declarations and new declarations for this name.
+    // Some of the declarations may simply replace existing ones. Handle those
+    // first.
+    llvm::SmallVector<unsigned, 8> Skip;
+    for (unsigned I = 0, N = Decls.size(); I != N; ++I)
+      if (List.HandleRedeclaration(Decls[I]))
+        Skip.push_back(I);
+    Skip.push_back(Decls.size());
+
+    // Add in any new declarations.
+    unsigned SkipPos = 0;
+    for (unsigned I = 0, N = Decls.size(); I != N; ++I) {
+      if (I == Skip[SkipPos])
+        ++SkipPos;
+      else
+        List.AddSubsequentDecl(Decls[I]);
+    }
+  } else {
+    // Convert the array to a StoredDeclsList.
+    for (ArrayRef<NamedDecl*>::iterator
+           I = Decls.begin(), E = Decls.end(); I != E; ++I) {
+      if (List.isNull())
+        List.setOnlyValue(*I);
+      else
+        List.AddSubsequentDecl(*I);
+    }
   }
 
   return List.getLookupResult();
diff --git a/test/PCH/cxx-namespaces.cpp b/test/PCH/cxx-namespaces.cpp
index e0ff27c020..e0feaab691 100644
--- a/test/PCH/cxx-namespaces.cpp
+++ b/test/PCH/cxx-namespaces.cpp
@@ -3,10 +3,23 @@
 
 // Test with pch.
 // RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/cxx-namespaces.h
-// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -ast-dump -ast-dump-lookups -ast-dump-filter N %s | FileCheck %s
+
+// Test with modules.
+// RUN: %clang_cc1 -fmodules -x c++-header -emit-pch -o %t %S/cxx-namespaces.h
+// RUN: %clang_cc1 -fmodules -include-pch %t -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fmodules -include-pch %t -fsyntax-only -ast-dump -ast-dump-lookups -ast-dump-filter N %s | FileCheck %s
 
 // expected-no-diagnostics
 
 void m() {
   N::x = 0;
+  N::f();
 }
+
+// namespace 'N' should contain only two declarations of 'f'.
+
+// CHECK:      DeclarationName 'f'
+// CHECK-NEXT: |-Function {{.*}} 'f' 'void (
+// CHECK-NEXT: `-Function {{.*}} 'f' 'void (
diff --git a/test/PCH/cxx-namespaces.h b/test/PCH/cxx-namespaces.h
index f3389533d0..26d75a079d 100644
--- a/test/PCH/cxx-namespaces.h
+++ b/test/PCH/cxx-namespaces.h
@@ -4,4 +4,7 @@ namespace N {
     namespace {
         int x;
     }
+
+    void f();
+    void f(int);
 }