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

@@ -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();

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 (

clang/test/PCH/cxx-namespaces.h

@@ -4,4 +4,7 @@ namespace N {
     namespace {
         int x;
     }
+
+    void f();
+    void f(int);
 }