Implement the all_lookups_iterator for PCH as a follow-up to r153970. This

includes a patch from Matthias Kleine with a regression testcase!

Adds a new iterator 'data_iterator' to OnDiskHashTable which doesn't try to
reconstruct the external_key from the internal_key, which is useful for traits
that don't store enough information to do that mapping in their key. Also
deletes the 'item_iterator' from OnDiskHashTable as dead code.


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

10 files changed, 125 insertions, 87 deletions

diff --git a/include/clang/AST/DeclLookups.h b/include/clang/AST/DeclLookups.h
index 66d190f429..b8abe97be1 100644
--- a/include/clang/AST/DeclLookups.h
+++ b/include/clang/AST/DeclLookups.h
@@ -67,6 +67,8 @@ public:
 
 DeclContext::all_lookups_iterator DeclContext::lookups_begin() const {
   DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
+  if (hasExternalVisibleStorage())
+    getParentASTContext().getExternalSource()->completeVisibleDeclsMap(Primary);
   if (StoredDeclsMap *Map = Primary->buildLookup())
     return all_lookups_iterator(Map->begin(), Map->end());
   return all_lookups_iterator();
@@ -74,6 +76,8 @@ DeclContext::all_lookups_iterator DeclContext::lookups_begin() const {
 
 DeclContext::all_lookups_iterator DeclContext::lookups_end() const {
   DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
+  if (hasExternalVisibleStorage())
+    getParentASTContext().getExternalSource()->completeVisibleDeclsMap(Primary);
   if (StoredDeclsMap *Map = Primary->buildLookup())
     return all_lookups_iterator(Map->end(), Map->end());
   return all_lookups_iterator();
diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h
index 18a1432b70..e2a60d5cf0 100644
--- a/include/clang/AST/ExternalASTSource.h
+++ b/include/clang/AST/ExternalASTSource.h
@@ -126,6 +126,12 @@ public:
   virtual DeclContextLookupResult
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// \brief Ensures that the table of all visible declarations inside this
+  /// context is up to date.
+  ///
+  /// The default implementation of this functino is a no-op.
+  virtual void completeVisibleDeclsMap(const DeclContext *DC);
+
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///
diff --git a/include/clang/Basic/OnDiskHashTable.h b/include/clang/Basic/OnDiskHashTable.h
index 8028a73326..b92f1cf9c6 100644
--- a/include/clang/Basic/OnDiskHashTable.h
+++ b/include/clang/Basic/OnDiskHashTable.h
@@ -132,7 +132,7 @@ class OnDiskChainedHashTableGenerator {
   class Bucket {
   public:
     io::Offset off;
-    Item*  head;
+    Item* head;
     unsigned length;
 
     Bucket() {}
@@ -201,6 +201,7 @@ public:
 
       // Write out the number of items in the bucket.
       Emit16(out, B.length);
+      assert(B.length != 0  && "Bucket has a head but zero length?");
 
       // Write out the entries in the bucket.
       for (Item *I = B.head; I ; I = I->next) {
@@ -398,31 +399,30 @@ public:
   }
   key_iterator key_end() { return key_iterator(); }
 
-  /// \brief Iterates over all the entries in the table, returning
-  /// a key/data pair.
-  class item_iterator {
+  /// \brief Iterates over all the entries in the table, returning the data.
+  class data_iterator {
     const unsigned char* Ptr;
     unsigned NumItemsInBucketLeft;
     unsigned NumEntriesLeft;
     Info *InfoObj;
   public:
-    typedef std::pair<external_key_type, data_type> value_type;
+    typedef data_type value_type;
 
-    item_iterator(const unsigned char* const Ptr, unsigned NumEntries,
+    data_iterator(const unsigned char* const Ptr, unsigned NumEntries,
                   Info *InfoObj)
       : Ptr(Ptr), NumItemsInBucketLeft(0), NumEntriesLeft(NumEntries),
         InfoObj(InfoObj) { }
-    item_iterator()
+    data_iterator()
       : Ptr(0), NumItemsInBucketLeft(0), NumEntriesLeft(0), InfoObj(0) { }
 
-    bool operator==(const item_iterator& X) const {
+    bool operator==(const data_iterator& X) const {
       return X.NumEntriesLeft == NumEntriesLeft;
     }
-    bool operator!=(const item_iterator& X) const {
+    bool operator!=(const data_iterator& X) const {
       return X.NumEntriesLeft != NumEntriesLeft;
     }
 
-    item_iterator& operator++() {  // Preincrement
+    data_iterator& operator++() {  // Preincrement
       if (!NumItemsInBucketLeft) {
         // 'Items' starts with a 16-bit unsigned integer representing the
         // number of items in this bucket.
@@ -438,8 +438,8 @@ public:
       --NumEntriesLeft;
       return *this;
     }
-    item_iterator operator++(int) {  // Postincrement
-      item_iterator tmp = *this; ++*this; return tmp;
+    data_iterator operator++(int) {  // Postincrement
+      data_iterator tmp = *this; ++*this; return tmp;
     }
 
     value_type operator*() const {
@@ -454,15 +454,14 @@ public:
       // Read the key.
       const internal_key_type& Key =
         InfoObj->ReadKey(LocalPtr, L.first);
-      return std::make_pair(InfoObj->GetExternalKey(Key),
-                          InfoObj->ReadData(Key, LocalPtr + L.first, L.second));
+      return InfoObj->ReadData(Key, LocalPtr + L.first, L.second);
     }
   };
 
-  item_iterator item_begin() {
-    return item_iterator(Base + 4, getNumEntries(), &InfoObj);
+  data_iterator data_begin() {
+    return data_iterator(Base + 4, getNumEntries(), &InfoObj);
   }
-  item_iterator item_end() { return item_iterator(); }
+  data_iterator data_end() { return data_iterator(); }
 
   Info &getInfoObj() { return InfoObj; }
 
diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index a0bcecc93a..a9d0fc3f1e 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -1472,7 +1472,7 @@ public:
                      llvm::DenseMap<IdentifierInfo *, uint64_t>::iterator Pos);
 
   /// \brief Load all external visible decls in the given DeclContext.
-  void completeVisibleDeclsMap(DeclContext *DC);
+  void completeVisibleDeclsMap(const DeclContext *DC);
 
   /// \brief Retrieve the AST context that this AST reader supplements.
   ASTContext &getContext() { return Context; }
diff --git a/lib/AST/ExternalASTSource.cpp b/lib/AST/ExternalASTSource.cpp
index fd616dbc9d..6b9fe26ccc 100644
--- a/lib/AST/ExternalASTSource.cpp
+++ b/lib/AST/ExternalASTSource.cpp
@@ -49,7 +49,10 @@ ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
   return DeclContext::lookup_result();
 }
 
-ExternalLoadResult 
+void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {
+}
+
+ExternalLoadResult
 ExternalASTSource::FindExternalLexicalDecls(const DeclContext *DC,
                                             bool (*isKindWeWant)(Decl::Kind),
                                          SmallVectorImpl<Decl*> &Result) {
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 05fcb4ba9f..06b42f3ab1 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -27,7 +27,6 @@
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLocVisitor.h"
-#include "llvm/Support/SaveAndRestore.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PreprocessingRecord.h"
 #include "clang/Lex/Preprocessor.h"
@@ -46,6 +45,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/system_error.h"
 #include <algorithm>
 #include <iterator>
@@ -664,46 +664,6 @@ ASTDeclContextNameLookupTrait::GetInternalKey(
   return Key;
 }
 
-ASTDeclContextNameLookupTrait::external_key_type 
-ASTDeclContextNameLookupTrait::GetExternalKey(
-                                          const internal_key_type& Key) const {
-  ASTContext &Context = Reader.getContext();
-  switch (Key.Kind) {
-  case DeclarationName::Identifier:
-    return DeclarationName((IdentifierInfo*)Key.Data);
-
-  case DeclarationName::ObjCZeroArgSelector:
-  case DeclarationName::ObjCOneArgSelector:
-  case DeclarationName::ObjCMultiArgSelector:
-    return DeclarationName(Selector(Key.Data));
-
-  case DeclarationName::CXXConstructorName:
-    return Context.DeclarationNames.getCXXConstructorName(
-             Context.getCanonicalType(Reader.getLocalType(F, Key.Data)));
-
-  case DeclarationName::CXXDestructorName:
-    return Context.DeclarationNames.getCXXDestructorName(
-             Context.getCanonicalType(Reader.getLocalType(F, Key.Data)));
-
-  case DeclarationName::CXXConversionFunctionName:
-    return Context.DeclarationNames.getCXXConversionFunctionName(
-             Context.getCanonicalType(Reader.getLocalType(F, Key.Data)));
-
-  case DeclarationName::CXXOperatorName:
-    return Context.DeclarationNames.getCXXOperatorName(
-                                       (OverloadedOperatorKind)Key.Data);
-
-  case DeclarationName::CXXLiteralOperatorName:
-    return Context.DeclarationNames.getCXXLiteralOperatorName(
-                                                   (IdentifierInfo*)Key.Data);
-
-  case DeclarationName::CXXUsingDirective:
-    return DeclarationName::getUsingDirectiveName();
-  }
-
-  llvm_unreachable("Invalid Name Kind ?");
-}
-
 std::pair<unsigned, unsigned>
 ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char*& d) {
   using namespace clang::io;
@@ -749,7 +709,7 @@ ASTDeclContextNameLookupTrait::ReadKey(const unsigned char* d, unsigned) {
 ASTDeclContextNameLookupTrait::data_type 
 ASTDeclContextNameLookupTrait::ReadData(internal_key_type, 
                                         const unsigned char* d,
-                                      unsigned DataLen) {
+                                        unsigned DataLen) {
   using namespace clang::io;
   unsigned NumDecls = ReadUnalignedLE16(d);
   LE32DeclID *Start = (LE32DeclID *)d;
@@ -4973,48 +4933,95 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
 }
 
 namespace {
-  /// \brief ModuleFile visitor used to complete the visible decls map of a
+  /// \brief ModuleFile visitor used to retrieve all visible names in a
   /// declaration context.
-  class DeclContextVisibleDeclMapVisitor {
+  class DeclContextAllNamesVisitor {
     ASTReader &Reader;
-    DeclContext *DC;
+    llvm::SmallVectorImpl<const DeclContext *> &Contexts;
+    const DeclContext *DC;
+    llvm::DenseMap<DeclarationName, SmallVector<NamedDecl *, 8> > &Decls;
 
   public:
-    DeclContextVisibleDeclMapVisitor(ASTReader &Reader, DeclContext *DC)
-      : Reader(Reader), DC(DC) { }
+    DeclContextAllNamesVisitor(ASTReader &Reader,
+                               SmallVectorImpl<const DeclContext *> &Contexts,
+                               llvm::DenseMap<DeclarationName,
+                                           SmallVector<NamedDecl *, 8> > &Decls)
+      : Reader(Reader), Contexts(Contexts), Decls(Decls) { }
 
     static bool visit(ModuleFile &M, void *UserData) {
-      return static_cast<DeclContextVisibleDeclMapVisitor*>(UserData)->visit(M);
-    }
+      DeclContextAllNamesVisitor *This
+        = static_cast<DeclContextAllNamesVisitor *>(UserData);
 
-    bool visit(ModuleFile &M) {
       // Check whether we have any visible declaration information for
       // this context in this module.
-      ModuleFile::DeclContextInfosMap::iterator
-        Info = M.DeclContextInfos.find(DC);
-      if (Info == M.DeclContextInfos.end() || 
-          !Info->second.NameLookupTableData)
+      ModuleFile::DeclContextInfosMap::iterator Info;
+      bool FoundInfo = false;
+      for (unsigned I = 0, N = This->Contexts.size(); I != N; ++I) {
+        Info = M.DeclContextInfos.find(This->Contexts[I]);
+        if (Info != M.DeclContextInfos.end() &&
+            Info->second.NameLookupTableData) {
+          FoundInfo = true;
+          break;
+        }
+      }
+
+      if (!FoundInfo)
         return false;
-      
-      // Look for this name within this module.
+
       ASTDeclContextNameLookupTable *LookupTable =
         Info->second.NameLookupTableData;
-      for (ASTDeclContextNameLookupTable::key_iterator
-             I = LookupTable->key_begin(),
-             E = LookupTable->key_end(); I != E; ++I) {
-        DC->lookup(*I); // Force loading of the visible decls for the decl name.
+      bool FoundAnything = false;
+      for (ASTDeclContextNameLookupTable::data_iterator
+	     I = LookupTable->data_begin(), E = LookupTable->data_end();
+	   I != E; ++I) {
+        ASTDeclContextNameLookupTrait::data_type Data = *I;
+        for (; Data.first != Data.second; ++Data.first) {
+          NamedDecl *ND = This->Reader.GetLocalDeclAs<NamedDecl>(M,
+                                                                 *Data.first);
+          if (!ND)
+            continue;
+
+          // Record this declaration.
+          FoundAnything = true;
+          This->Decls[ND->getDeclName()].push_back(ND);
+        }
       }
 
-      return false;
+      return FoundAnything;
     }
   };
 }
 
-void ASTReader::completeVisibleDeclsMap(DeclContext *DC) {
+void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) {
   if (!DC->hasExternalVisibleStorage())
     return;
-  DeclContextVisibleDeclMapVisitor Visitor(*this, DC);
-  ModuleMgr.visit(&DeclContextVisibleDeclMapVisitor::visit, &Visitor);
+  llvm::DenseMap<DeclarationName, llvm::SmallVector<NamedDecl*, 8> > Decls;
+
+  // Compute the declaration contexts we need to look into. Multiple such
+  // declaration contexts occur when two declaration contexts from disjoint
+  // modules get merged, e.g., when two namespaces with the same name are
+  // independently defined in separate modules.
+  SmallVector<const DeclContext *, 2> Contexts;
+  Contexts.push_back(DC);
+
+  if (DC->isNamespace()) {
+    MergedDeclsMap::iterator Merged
+      = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
+    if (Merged != MergedDecls.end()) {
+      for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
+        Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
+    }
+  }
+
+  DeclContextAllNamesVisitor Visitor(*this, Contexts, Decls);
+  ModuleMgr.visit(&DeclContextAllNamesVisitor::visit, &Visitor);
+  ++NumVisibleDeclContextsRead;
+
+  for (llvm::DenseMap<DeclarationName,
+                      llvm::SmallVector<NamedDecl*, 8> >::iterator
+         I = Decls.begin(), E = Decls.end(); I != E; ++I) {
+    SetExternalVisibleDeclsForName(DC, I->first, I->second);
+  }
 }
 
 /// \brief Under non-PCH compilation the consumer receives the objc methods
diff --git a/lib/Serialization/ASTReaderInternals.h b/lib/Serialization/ASTReaderInternals.h
index da90c3400a..e5159e9526 100644
--- a/lib/Serialization/ASTReaderInternals.h
+++ b/lib/Serialization/ASTReaderInternals.h
@@ -57,8 +57,7 @@ public:
   typedef DeclarationName external_key_type;
   typedef DeclNameKey internal_key_type;
 
-  explicit ASTDeclContextNameLookupTrait(ASTReader &Reader, 
-                                         ModuleFile &F) 
+  explicit ASTDeclContextNameLookupTrait(ASTReader &Reader, ModuleFile &F)
     : Reader(Reader), F(F) { }
 
   static bool EqualKey(const internal_key_type& a,
@@ -68,9 +67,8 @@ public:
 
   unsigned ComputeHash(const DeclNameKey &Key) const;
   internal_key_type GetInternalKey(const external_key_type& Name) const;
-  external_key_type GetExternalKey(const internal_key_type& Key) const;
 
-  static std::pair<unsigned, unsigned> 
+  static std::pair<unsigned, unsigned>
   ReadKeyDataLength(const unsigned char*& d);
 
   internal_key_type ReadKey(const unsigned char* d, unsigned);
diff --git a/test/CodeCompletion/preamble.c b/test/CodeCompletion/preamble.c
new file mode 100644
index 0000000000..98ad67976c
--- /dev/null
+++ b/test/CodeCompletion/preamble.c
@@ -0,0 +1,7 @@
+#include "some_struct.h"
+void foo() {
+  struct X x;
+  x.
+
+// RUN: CINDEXTEST_EDITING=1 c-index-test -code-completion-at=%s:4:5 -Xclang -code-completion-patterns  %s | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: FieldDecl:{ResultType int}{TypedText m} (35) (parent: StructDecl 'X')
diff --git a/test/CodeCompletion/some_struct.h b/test/CodeCompletion/some_struct.h
new file mode 100644
index 0000000000..63aa2a39d3
--- /dev/null
+++ b/test/CodeCompletion/some_struct.h
@@ -0,0 +1 @@
+struct X { int m; };
diff --git a/test/PCH/typo2.cpp b/test/PCH/typo2.cpp
new file mode 100644
index 0000000000..f9b4c833e0
--- /dev/null
+++ b/test/PCH/typo2.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -emit-pch %s -o %t.pch
+// RUN: %clang_cc1 -include-pch %t.pch %s -verify
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+void func(struct Test);  // expected-note{{'Test' declared here}}
+
+#else
+
+::Yest *T;  // expected-error{{did you mean 'Test'}}
+
+#endif