From 51445cd3078617c84e87d9c97b129f59aa906cc5 Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Mon, 24 Jun 2013 01:46:41 +0000
Subject: [PATCH] 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.

llvm-svn: 184679
---
 clang/lib/AST/DeclBase.cpp        | 39 +++++++++++++++++++++++++------
 clang/test/PCH/cxx-namespaces.cpp | 15 +++++++++++-
 clang/test/PCH/cxx-namespaces.h   |  3 +++
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 084a4321d8f1..fb13766ad0ae 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/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/clang/test/PCH/cxx-namespaces.cpp b/clang/test/PCH/cxx-namespaces.cpp
index e0ff27c020c4..e0feaab6910a 100644
--- a/clang/test/PCH/cxx-namespaces.cpp
+++ b/clang/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/clang/test/PCH/cxx-namespaces.h b/clang/test/PCH/cxx-namespaces.h
index f3389533d001..26d75a079d92 100644
--- a/clang/test/PCH/cxx-namespaces.h
+++ b/clang/test/PCH/cxx-namespaces.h
@@ -4,4 +4,7 @@ namespace N {
     namespace {
         int x;
     }
+
+    void f();
+    void f(int);
 }