Make the loading of information attached to an IdentifierInfo from an

AST file more lazy, so that we don't eagerly load that information for
all known identifiers each time a new AST file is loaded. The eager
reloading made some sense in the context of precompiled headers, since
very few identifiers were defined before PCH load time. With modules,
however, a huge amount of code can get parsed before we see an
@import, so laziness becomes important here.

The approach taken to make this information lazy is fairly simple:
when we load a new AST file, we mark all of the existing identifiers
as being out-of-date. Whenever we want to access information that may
come from an AST (e.g., whether the identifier has a macro definition,
or what top-level declarations have that name), we check the
out-of-date bit and, if it's set, ask the AST reader to update the
IdentifierInfo from the AST files. The update is a merge, and we now
take care to merge declarations before/after imports with declarations
from multiple imports.

The results of this optimization are fairly dramatic. On a small
application that brings in 14 non-trivial modules, this takes modules
from being > 3x slower than a "perfect" PCH file down to 30% slower
for a full rebuild. A partial rebuild (where the PCH file or modules
can be re-used) is down to 7% slower. Making the PCH file just a
little imperfect (e.g., adding two smallish modules used by a bunch of
.m files that aren't in the PCH file) tips the scales in favor of the
modules approach, with 24% faster partial rebuilds.

This is just a first step; the lazy scheme could possibly be improved
by adding versioning, so we don't search into modules we already
searched. Moreover, we'll need similar lazy schemes for all of the
other lookup data structures, such as DeclContexts.



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

include/clang/Basic/IdentifierTable.h

@@ -49,8 +49,6 @@ namespace clang {
 /// variable or function name).  The preprocessor keeps this information in a
 /// set, and all tok::identifier tokens have a pointer to one of these.
 class IdentifierInfo {
-  // Note: DON'T make TokenID a 'tok::TokenKind'; MSVC will treat it as a
-  //       signed char and TokenKinds > 255 won't be handled correctly.
   unsigned TokenID            : 9; // Front-end token ID or tok::identifier.
   // Objective-C keyword ('protocol' in '@protocol') or builtin (__builtin_inf).
   // First NUM_OBJC_KEYWORDS values are for Objective-C, the remaining values
@@ -62,11 +60,17 @@ class IdentifierInfo {
   bool IsPoisoned             : 1; // True if identifier is poisoned.
   bool IsCPPOperatorKeyword   : 1; // True if ident is a C++ operator keyword.
   bool NeedsHandleIdentifier  : 1; // See "RecomputeNeedsHandleIdentifier".
-  bool IsFromAST              : 1; // True if identfier first appeared in an AST
-                                   // file and wasn't modified since.
+  bool IsFromAST              : 1; // True if identifier was loaded (at least 
+                                   // partially) from an AST file.
+  bool ChangedAfterLoad       : 1; // True if identifier has changed from the
+                                   // definition loaded from an AST file.
   bool RevertedTokenID        : 1; // True if RevertTokenIDToIdentifier was
                                    // called.
-  // 5 bits left in 32-bit word.
+  bool OutOfDate              : 1; // True if there may be additional
+                                   // information about this identifier
+                                   // stored externally.
+  // 2 bits left in 32-bit word.
+  
   void *FETokenInfo;               // Managed by the language front-end.
   llvm::StringMapEntry<IdentifierInfo*> *Entry;
 
@@ -132,7 +136,6 @@ public:
       NeedsHandleIdentifier = 1;
     else
       RecomputeNeedsHandleIdentifier();
-    IsFromAST = false;
   }
 
   /// getTokenID - If this is a source-language token (e.g. 'for'), this API
@@ -221,7 +224,6 @@ public:
       NeedsHandleIdentifier = 1;
     else
       RecomputeNeedsHandleIdentifier();
-    IsFromAST = false;
   }
 
   /// isPoisoned - Return true if this token has been poisoned.
@@ -253,8 +255,34 @@ public:
   /// from an AST file.
   bool isFromAST() const { return IsFromAST; }
 
-  void setIsFromAST(bool FromAST = true) { IsFromAST = FromAST; }
+  void setIsFromAST() { IsFromAST = true; }
 
+  /// \brief Determine whether this identifier has changed since it was loaded
+  /// from an AST file.
+  bool hasChangedSinceDeserialization() const {
+    return ChangedAfterLoad;
+  }
+  
+  /// \brief Note that this identifier has changed since it was loaded from
+  /// an AST file.
+  void setChangedSinceDeserialization() {
+    ChangedAfterLoad = true;
+  }
+
+  /// \brief Determine whether the information for this identifier is out of
+  /// date with respect to the external source.
+  bool isOutOfDate() const { return OutOfDate; }
+  
+  /// \brief Set whether the information for this identifier is out of
+  /// date with respect to the external source.
+  void setOutOfDate(bool OOD) {
+    OutOfDate = OOD;
+    if (OOD)
+      NeedsHandleIdentifier = true;
+    else
+      RecomputeNeedsHandleIdentifier();
+  }
+  
 private:
   /// RecomputeNeedsHandleIdentifier - The Preprocessor::HandleIdentifier does
   /// several special (but rare) things to identifiers of various sorts.  For
@@ -266,7 +294,7 @@ private:
   void RecomputeNeedsHandleIdentifier() {
     NeedsHandleIdentifier =
       (isPoisoned() | hasMacroDefinition() | isCPlusPlusOperatorKeyword() |
-       isExtensionToken() | isCXX11CompatKeyword() ||
+       isExtensionToken() | isCXX11CompatKeyword() || isOutOfDate() ||
        (getTokenID() == tok::kw___import_module__));
   }
 };

include/clang/Lex/ExternalPreprocessorSource.h

@@ -30,6 +30,9 @@ public:
   
   /// \brief Read the definition for the given macro.
   virtual void LoadMacroDefinition(IdentifierInfo *II) = 0;
+  
+  /// \brief Update an out-of-date identifier.
+  virtual void updateOutOfDateIdentifier(IdentifierInfo &II) = 0;
 };
   
 }

include/clang/Sema/IdentifierResolver.h

@@ -23,9 +23,11 @@ class ASTContext;
 class Decl;
 class DeclContext;
 class DeclarationName;
+class ExternalPreprocessorSource;
 class NamedDecl;
+class Preprocessor;
 class Scope;
-
+  
 /// IdentifierResolver - Keeps track of shadowed decls on enclosing
 /// scopes.  It manages the shadowing chains of declaration names and
 /// implements efficient decl lookup based on a declaration name.
@@ -141,10 +143,10 @@ public:
   };
 
   /// begin - Returns an iterator for decls with the name 'Name'.
-  static iterator begin(DeclarationName Name);
+  iterator begin(DeclarationName Name);
 
   /// end - Returns an iterator that has 'finished'.
-  static iterator end() {
+  iterator end() {
     return iterator();
   }
 
@@ -175,23 +177,29 @@ public:
   /// position.
   void InsertDeclAfter(iterator Pos, NamedDecl *D);
 
-  /// \brief Link the declaration into the chain of declarations for
-  /// the given identifier.
+  /// \brief Try to add the given declaration to the top level scope, if it
+  /// (or a redeclaration of it) hasn't already been added.
   ///
-  /// This is a lower-level routine used by the AST reader to link a
-  /// declaration into a specific IdentifierInfo before the
-  /// declaration actually has a name.
-  void AddDeclToIdentifierChain(IdentifierInfo *II, NamedDecl *D);
-
-  explicit IdentifierResolver(const LangOptions &LangOpt);
+  /// \param D The externally-produced declaration to add.
+  ///
+  /// \param Name The name of the externally-produced declaration.
+  ///
+  /// \returns true if the declaration was added, false otherwise.
+  bool tryAddTopLevelDecl(NamedDecl *D, DeclarationName Name);
+  
+  explicit IdentifierResolver(Preprocessor &PP);
   ~IdentifierResolver();
 
 private:
   const LangOptions &LangOpt;
-
+  Preprocessor &PP;
+  
   class IdDeclInfoMap;
   IdDeclInfoMap *IdDeclInfos;
 
+  void updatingIdentifier(IdentifierInfo &II);
+  void readingIdentifier(IdentifierInfo &II);
+  
   /// FETokenInfo contains a Decl pointer if lower bit == 0.
   static inline bool isDeclPtr(void *Ptr) {
     return (reinterpret_cast<uintptr_t>(Ptr) & 0x1) == 0;

include/clang/Sema/Sema.h

@@ -1302,6 +1302,14 @@ public:
   /// Add this decl to the scope shadowed decl chains.
   void PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext = true);
 
+  /// \brief Make the given externally-produced declaration visible at the
+  /// top level scope.
+  ///
+  /// \param D The externally-produced declaration to push.
+  ///
+  /// \param Name The name of the externally-produced declaration.
+  void pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name);
+  
   /// isDeclInScope - If 'Ctx' is a function/method, isDeclInScope returns true
   /// if 'D' is in Scope 'S', otherwise 'S' is ignored and isDeclInScope returns
   /// true if 'D' belongs to the given declaration context.

include/clang/Serialization/ASTReader.h

@@ -1256,6 +1256,9 @@ public:
   /// \brief Read the macro definition for this identifier.
   virtual void LoadMacroDefinition(IdentifierInfo *II);
 
+  /// \brief Update an out-of-date identifier.
+  virtual void updateOutOfDateIdentifier(IdentifierInfo &II);
+
   /// \brief Read the macro definition corresponding to this iterator
   /// into the unread macro record offsets table.
   void LoadMacroDefinition(

include/clang/Serialization/ASTWriter.h

@@ -44,6 +44,7 @@ class CXXBaseSpecifier;
 class CXXCtorInitializer;
 class FPOptions;
 class HeaderSearch;
+class IdentifierResolver;
 class MacroDefinition;
 class MemorizeStatCalls;
 class OpaqueValueExpr;
@@ -355,7 +356,8 @@ private:
   void WriteTypeDeclOffsets();
   void WriteSelectors(Sema &SemaRef);
   void WriteReferencedSelectorsPool(Sema &SemaRef);
-  void WriteIdentifierTable(Preprocessor &PP, bool IsModule);
+  void WriteIdentifierTable(Preprocessor &PP, IdentifierResolver &IdResolver,
+                            bool IsModule);
   void WriteAttributes(const AttrVec &Attrs, RecordDataImpl &Record);
   void ResolveDeclUpdatesBlocks();
   void WriteDeclUpdatesBlocks();

lib/Basic/IdentifierTable.cpp

@@ -37,7 +37,9 @@ IdentifierInfo::IdentifierInfo() {
   IsCPPOperatorKeyword = false;
   NeedsHandleIdentifier = false;
   IsFromAST = false;
+  ChangedAfterLoad = false;
   RevertedTokenID = false;
+  OutOfDate = false;
   FETokenInfo = 0;
   Entry = 0;
 }

lib/Lex/PPMacroExpansion.cpp

@@ -51,9 +51,13 @@ void Preprocessor::setMacroInfo(IdentifierInfo *II, MacroInfo *MI) {
   if (MI) {
     Macros[II] = MI;
     II->setHasMacroDefinition(true);
+    if (II->isFromAST())
+      II->setChangedSinceDeserialization();
   } else if (II->hasMacroDefinition()) {
     Macros.erase(II);
     II->setHasMacroDefinition(false);
+    if (II->isFromAST())
+      II->setChangedSinceDeserialization();
   }
 }
 

lib/Lex/Pragma.cpp

@@ -304,6 +304,8 @@ void Preprocessor::HandlePragmaPoison(Token &PoisonTok) {
 
     // Finally, poison it!
     II->setIsPoisoned();
+    if (II->isFromAST())
+      II->setChangedSinceDeserialization();
   }
 }
 

lib/Lex/Preprocessor.cpp

@@ -492,6 +492,13 @@ void Preprocessor::HandleIdentifier(Token &Identifier) {
 
   IdentifierInfo &II = *Identifier.getIdentifierInfo();
 
+  // If the information about this identifier is out of date, update it from
+  // the external source.
+  if (II.isOutOfDate()) {
+    ExternalSource->updateOutOfDateIdentifier(II);
+    Identifier.setKind(II.getTokenID());
+  }
+  
   // If this identifier was poisoned, and if it was not produced from a macro
   // expansion, emit an error.
   if (II.isPoisoned() && CurPPLexer) {

lib/Sema/IdentifierResolver.cpp

@@ -15,7 +15,10 @@
 #include "clang/Sema/IdentifierResolver.h"
 #include "clang/Sema/Scope.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Lex/ExternalPreprocessorSource.h"
+#include "clang/Lex/Preprocessor.h"
 
 using namespace clang;
 
@@ -93,9 +96,11 @@ IdentifierResolver::IdDeclInfo::ReplaceDecl(NamedDecl *Old, NamedDecl *New) {
 // IdentifierResolver Implementation
 //===----------------------------------------------------------------------===//
 
-IdentifierResolver::IdentifierResolver(const LangOptions &langOpt)
-    : LangOpt(langOpt), IdDeclInfos(new IdDeclInfoMap) {
+IdentifierResolver::IdentifierResolver(Preprocessor &PP)
+  : LangOpt(PP.getLangOptions()), PP(PP),
+    IdDeclInfos(new IdDeclInfoMap) {
 }
+
 IdentifierResolver::~IdentifierResolver() {
   delete IdDeclInfos;
 }
@@ -146,7 +151,7 @@ bool IdentifierResolver::isDeclInScope(Decl *D, DeclContext *Ctx,
 void IdentifierResolver::AddDecl(NamedDecl *D) {
   DeclarationName Name = D->getDeclName();
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
-    II->setIsFromAST(false);
+    updatingIdentifier(*II);
 
   void *Ptr = Name.getFETokenInfo<void>();
 
@@ -170,6 +175,9 @@ void IdentifierResolver::AddDecl(NamedDecl *D) {
 
 void IdentifierResolver::InsertDeclAfter(iterator Pos, NamedDecl *D) {
   DeclarationName Name = D->getDeclName();
+  if (IdentifierInfo *II = Name.getAsIdentifierInfo())
+    updatingIdentifier(*II);
+  
   void *Ptr = Name.getFETokenInfo<void>();
   
   if (!Ptr) {
@@ -195,7 +203,8 @@ void IdentifierResolver::InsertDeclAfter(iterator Pos, NamedDecl *D) {
   }
 
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
-    II->setIsFromAST(false);
+    if (II->isFromAST())
+      II->setChangedSinceDeserialization();
   
   // General case: insert the declaration at the appropriate point in the 
   // list, which already has at least two elements.
@@ -212,7 +221,7 @@ void IdentifierResolver::RemoveDecl(NamedDecl *D) {
   assert(D && "null param passed");
   DeclarationName Name = D->getDeclName();
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
-    II->setIsFromAST(false);
+    updatingIdentifier(*II);
 
   void *Ptr = Name.getFETokenInfo<void>();
 
@@ -233,7 +242,7 @@ bool IdentifierResolver::ReplaceDecl(NamedDecl *Old, NamedDecl *New) {
 
   DeclarationName Name = Old->getDeclName();
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
-    II->setIsFromAST(false);
+    updatingIdentifier(*II);
 
   void *Ptr = Name.getFETokenInfo<void>();
 
@@ -254,6 +263,9 @@ bool IdentifierResolver::ReplaceDecl(NamedDecl *Old, NamedDecl *New) {
 /// begin - Returns an iterator for decls with name 'Name'.
 IdentifierResolver::iterator
 IdentifierResolver::begin(DeclarationName Name) {
+  if (IdentifierInfo *II = Name.getAsIdentifierInfo())
+    readingIdentifier(*II);
+    
   void *Ptr = Name.getFETokenInfo<void>();
   if (!Ptr) return end();
 
@@ -269,27 +281,142 @@ IdentifierResolver::begin(DeclarationName Name) {
   return end();
 }
 
-void IdentifierResolver::AddDeclToIdentifierChain(IdentifierInfo *II,
-                                                  NamedDecl *D) {
-  II->setIsFromAST(false);
-  void *Ptr = II->getFETokenInfo<void>();
+namespace {
+  enum DeclMatchKind {
+    DMK_Different,
+    DMK_Replace,
+    DMK_Ignore
+  };
+}
 
-  if (!Ptr) {
-    II->setFETokenInfo(D);
-    return;
+/// \brief Compare two declarations to see whether they are different or,
+/// if they are the same, whether the new declaration should replace the 
+/// existing declaration.
+static DeclMatchKind compareDeclarations(NamedDecl *Existing, NamedDecl *New) {
+  // If the declarations are identical, ignore the new one.
+  if (Existing == New)
+    return DMK_Ignore;
+
+  // If the declarations have different kinds, they're obviously different.
+  if (Existing->getKind() != New->getKind())
+    return DMK_Different;
+
+  // If the declarations are redeclarations of each other, keep the newest one.
+  if (Existing->getCanonicalDecl() == New->getCanonicalDecl()) {
+    // If the existing declaration is somewhere in the previous declaration
+    // chain of the new declaration, then prefer the new declaration.
+    for (Decl::redecl_iterator RD = New->redecls_begin(), 
+                            RDEnd = New->redecls_end();
+         RD != RDEnd; ++RD) {
+      if (*RD == Existing)
+        return DMK_Replace;
+        
+      if (RD->isCanonicalDecl())
+        break;
+    }
+    
+    return DMK_Ignore;
   }
+  
+  // If the declarations are both Objective-C classes, and one is a forward
+  // declaration and the other is not, take the full definition.
+  // FIXME: At some point, we'll actually have to detect collisions better.
+  // This logic, however, belongs in the AST reader, not here.
+  if (ObjCInterfaceDecl *ExistingIFace = dyn_cast<ObjCInterfaceDecl>(Existing))
+    if (ObjCInterfaceDecl *NewIFace = dyn_cast<ObjCInterfaceDecl>(New))
+      if (ExistingIFace->isForwardDecl() != NewIFace->isForwardDecl())
+        return ExistingIFace->isForwardDecl()? DMK_Replace : DMK_Ignore;
+        
+  return DMK_Different;
+}
 
+bool IdentifierResolver::tryAddTopLevelDecl(NamedDecl *D, DeclarationName Name){
+  if (IdentifierInfo *II = Name.getAsIdentifierInfo())
+    updatingIdentifier(*II);
+  
+  void *Ptr = Name.getFETokenInfo<void>();
+    
+  if (!Ptr) {
+    Name.setFETokenInfo(D);
+    return true;
+  }
+  
   IdDeclInfo *IDI;
-
+  
   if (isDeclPtr(Ptr)) {
-    II->setFETokenInfo(NULL);
-    IDI = &(*IdDeclInfos)[II];
     NamedDecl *PrevD = static_cast<NamedDecl*>(Ptr);
-    IDI->AddDecl(PrevD);
-  } else
-    IDI = toIdDeclInfo(Ptr);
+    
+    switch (compareDeclarations(PrevD, D)) {
+    case DMK_Different:
+      break;
+      
+    case DMK_Ignore:
+      return false;
+      
+    case DMK_Replace:
+      Name.setFETokenInfo(D);
+      return true;
+    }
+    
+    Name.setFETokenInfo(NULL);
+    IDI = &(*IdDeclInfos)[Name];
+    
+    // If the existing declaration is not visible in translation unit scope,
+    // then add the new top-level declaration first.
+    if (!PrevD->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
+      IDI->AddDecl(D);
+      IDI->AddDecl(PrevD);
+    } else {
+      IDI->AddDecl(PrevD);
+      IDI->AddDecl(D);
+    }
+    return true;
+  } 
+  
+  IDI = toIdDeclInfo(Ptr);
 
+  // See whether this declaration is identical to any existing declarations.
+  // If not, find the right place to insert it.
+  for (IdDeclInfo::DeclsTy::iterator I = IDI->decls_begin(), 
+                                  IEnd = IDI->decls_end();
+       I != IEnd; ++I) {
+    
+    switch (compareDeclarations(*I, D)) {
+    case DMK_Different:
+      break;
+      
+    case DMK_Ignore:
+      return false;
+      
+    case DMK_Replace:
+      *I = D;
+      return true;
+    }
+    
+    if (!(*I)->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
+      // We've found a declaration that is not visible from the translation
+      // unit (it's in an inner scope). Insert our declaration here.
+      IDI->InsertDecl(I, D);
+      return true;
+    }
+  }
+  
+  // Add the declaration to the end.
   IDI->AddDecl(D);
+  return true;
+}
+
+void IdentifierResolver::readingIdentifier(IdentifierInfo &II) {
+  if (II.isOutOfDate())
+    PP.getExternalSource()->updateOutOfDateIdentifier(II);  
+}
+
+void IdentifierResolver::updatingIdentifier(IdentifierInfo &II) {
+  if (II.isOutOfDate())
+    PP.getExternalSource()->updateOutOfDateIdentifier(II);
+  
+  if (II.isFromAST())
+    II.setChangedSinceDeserialization();
 }
 
 //===----------------------------------------------------------------------===//

lib/Sema/Sema.cpp

@@ -98,7 +98,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     CurContext(0), OriginalLexicalContext(0),
     PackContext(0), MSStructPragmaOn(false), VisContext(0),
     ExprNeedsCleanups(0), LateTemplateParser(0), OpaqueParser(0),
-    IdResolver(pp.getLangOptions()), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
+    IdResolver(pp), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
     GlobalNewDeleteDeclared(false), 
     ObjCShouldCallSuperDealloc(false),
     ObjCShouldCallSuperFinalize(false),
@@ -143,11 +143,11 @@ void Sema::Initialize() {
     // If either of the 128-bit integer types are unavailable to name lookup,
     // define them now.
     DeclarationName Int128 = &Context.Idents.get("__int128_t");
-    if (IdentifierResolver::begin(Int128) == IdentifierResolver::end())
+    if (IdResolver.begin(Int128) == IdResolver.end())
       PushOnScopeChains(Context.getInt128Decl(), TUScope);
 
     DeclarationName UInt128 = &Context.Idents.get("__uint128_t");
-    if (IdentifierResolver::begin(UInt128) == IdentifierResolver::end())
+    if (IdResolver.begin(UInt128) == IdResolver.end())
       PushOnScopeChains(Context.getUInt128Decl(), TUScope);
   }
   
@@ -157,18 +157,18 @@ void Sema::Initialize() {
     // If 'SEL' does not yet refer to any declarations, make it refer to the
     // predefined 'SEL'.
     DeclarationName SEL = &Context.Idents.get("SEL");
-    if (IdentifierResolver::begin(SEL) == IdentifierResolver::end())
+    if (IdResolver.begin(SEL) == IdResolver.end())
       PushOnScopeChains(Context.getObjCSelDecl(), TUScope);
 
     // If 'id' does not yet refer to any declarations, make it refer to the
     // predefined 'id'.
     DeclarationName Id = &Context.Idents.get("id");
-    if (IdentifierResolver::begin(Id) == IdentifierResolver::end())
+    if (IdResolver.begin(Id) == IdResolver.end())
       PushOnScopeChains(Context.getObjCIdDecl(), TUScope);
     
     // Create the built-in typedef for 'Class'.
     DeclarationName Class = &Context.Idents.get("Class");
-    if (IdentifierResolver::begin(Class) == IdentifierResolver::end())
+    if (IdResolver.begin(Class) == IdResolver.end())
       PushOnScopeChains(Context.getObjCClassDecl(), TUScope);
   }
 }

lib/Sema/SemaDecl.cpp

@@ -938,6 +938,11 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext) {
   }
 }
 
+void Sema::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
+  if (IdResolver.tryAddTopLevelDecl(D, Name) && TUScope)
+    TUScope->AddDecl(D);
+}
+
 bool Sema::isDeclInScope(NamedDecl *&D, DeclContext *Ctx, Scope *S,
                          bool ExplicitInstantiationOrSpecialization) {
   return IdResolver.isDeclInScope(D, Ctx, Context, S,

lib/Serialization/ASTReader.cpp

@@ -514,6 +514,7 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
       II = &Reader.getIdentifierTable().getOwn(StringRef(k.first, k.second));
     Reader.SetIdentifierInfo(ID, II);
     II->setIsFromAST();
+    II->setOutOfDate(false);
     return II;
   }
 
@@ -539,7 +540,8 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
   IdentifierInfo *II = KnownII;
   if (!II)
     II = &Reader.getIdentifierTable().getOwn(StringRef(k.first, k.second));
-  Reader.SetIdentifierInfo(ID, II);
+  II->setOutOfDate(false);
+  II->setIsFromAST();
 
   // Set or check the various bits in the IdentifierInfo structure.
   // Token IDs are read-only.
@@ -564,6 +566,8 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
     DataLen -= 4;
   }
 
+  Reader.SetIdentifierInfo(ID, II);
+
   // Read all of the declarations visible at global scope with this
   // name.
   if (DataLen > 0) {
@@ -573,7 +577,6 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
     Reader.SetGloballyVisibleDecls(II, DeclIDs);
   }
 
-  II->setIsFromAST();
   return II;
 }
 
@@ -1276,6 +1279,7 @@ void ASTReader::ReadMacroRecord(Module &F, uint64_t Offset) {
         Error("macro must have a name in AST file");
         return;
       }
+      
       SourceLocation Loc = ReadSourceLocation(F, Record[1]);
       bool isUsed = Record[2];
 
@@ -1500,6 +1504,46 @@ void ASTReader::LoadMacroDefinition(IdentifierInfo *II) {
   LoadMacroDefinition(Pos);
 }
 
+namespace {
+  /// \brief Visitor class used to look up identifirs in an AST file.
+  class IdentifierLookupVisitor {
+    StringRef Name;
+    IdentifierInfo *Found;
+  public:
+    explicit IdentifierLookupVisitor(StringRef Name) : Name(Name), Found() { }
+    
+    static bool visit(Module &M, void *UserData) {
+      IdentifierLookupVisitor *This
+        = static_cast<IdentifierLookupVisitor *>(UserData);
+      
+      ASTIdentifierLookupTable *IdTable
+        = (ASTIdentifierLookupTable *)M.IdentifierLookupTable;
+      if (!IdTable)
+        return false;
+      
+      std::pair<const char*, unsigned> Key(This->Name.begin(), 
+                                           This->Name.size());
+      ASTIdentifierLookupTable::iterator Pos = IdTable->find(Key);
+      if (Pos == IdTable->end())
+        return false;
+      
+      // Dereferencing the iterator has the effect of building the
+      // IdentifierInfo node and populating it with the various
+      // declarations it needs.
+      This->Found = *Pos;
+      return true;
+    }
+    
+    // \brief Retrieve the identifier info found within the module
+    // files.
+    IdentifierInfo *getIdentifierInfo() const { return Found; }
+  };
+}
+
+void ASTReader::updateOutOfDateIdentifier(IdentifierInfo &II) {
+  get(II.getName());
+}
+
 const FileEntry *ASTReader::getFileEntry(StringRef filenameStrRef) {
   std::string Filename = filenameStrRef;
   MaybeAddSystemRootToFilename(Filename);
@@ -2329,43 +2373,6 @@ ASTReader::ASTReadResult ASTReader::validateFileEntries(Module &M) {
   return Success;
 }
 
-namespace {
-  /// \brief Visitor class used to look up identifirs in an AST file.
-  class IdentifierLookupVisitor {
-    StringRef Name;
-    IdentifierInfo *Found;
-  public:
-    explicit IdentifierLookupVisitor(StringRef Name) : Name(Name), Found() { }
-    
-    static bool visit(Module &M, void *UserData) {
-      IdentifierLookupVisitor *This
-      = static_cast<IdentifierLookupVisitor *>(UserData);
-      
-      ASTIdentifierLookupTable *IdTable
-        = (ASTIdentifierLookupTable *)M.IdentifierLookupTable;
-      if (!IdTable)
-        return false;
-      
-      std::pair<const char*, unsigned> Key(This->Name.begin(), 
-                                           This->Name.size());
-      ASTIdentifierLookupTable::iterator Pos = IdTable->find(Key);
-      if (Pos == IdTable->end())
-        return false;
-      
-      // Dereferencing the iterator has the effect of building the
-      // IdentifierInfo node and populating it with the various
-      // declarations it needs.
-      This->Found = *Pos;
-      return true;
-    }
-    
-    // \brief Retrieve the identifier info found within the module
-    // files.
-    IdentifierInfo *getIdentifierInfo() const { return Found; }
-  };
-}
-
-
 ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
                                             ModuleKind Type) {
   switch(ReadASTCore(FileName, Type, /*ImportedBy=*/0)) {
@@ -2384,33 +2391,13 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
       CheckPredefinesBuffers())
     return IgnorePCH;
 
-  // Initialization of keywords and pragmas occurs before the
-  // AST file is read, so there may be some identifiers that were
-  // loaded into the IdentifierTable before we intercepted the
-  // creation of identifiers. Iterate through the list of known
-  // identifiers and determine whether we have to establish
-  // preprocessor definitions or top-level identifier declaration
-  // chains for those identifiers.
-  //
-  // We copy the IdentifierInfo pointers to a small vector first,
-  // since de-serializing declarations or macro definitions can add
-  // new entries into the identifier table, invalidating the
-  // iterators.
-  //
-  // FIXME: We need a lazier way to load this information, e.g., by marking
-  // the identifier data as 'dirty', so that it will be looked up in the
-  // AST file(s) if it is uttered in the source. This could save us some
-  // module load time.
-  SmallVector<IdentifierInfo *, 128> Identifiers;
+  // Mark all of the identifiers in the identifier table as being out of date,
+  // so that various accessors know to check the loaded modules when the
+  // identifier is used.
   for (IdentifierTable::iterator Id = PP.getIdentifierTable().begin(),
                               IdEnd = PP.getIdentifierTable().end();
        Id != IdEnd; ++Id)
-    Identifiers.push_back(Id->second);
-  
-  for (unsigned I = 0, N = Identifiers.size(); I != N; ++I) {
-    IdentifierLookupVisitor Visitor(Identifiers[I]->getName());
-    ModuleMgr.visit(IdentifierLookupVisitor::visit, &Visitor);
-  }
+    Id->second->setOutOfDate(true);
 
   InitializeContext();
 
@@ -4412,10 +4399,8 @@ void ASTReader::InitializeSema(Sema &S) {
   // Makes sure any declarations that were deserialized "too early"
   // still get added to the identifier's declaration chains.
   for (unsigned I = 0, N = PreloadedDecls.size(); I != N; ++I) {
-    if (SemaObj->TUScope)
-      SemaObj->TUScope->AddDecl(PreloadedDecls[I]);
-
-    SemaObj->IdResolver.AddDecl(PreloadedDecls[I]);
+    SemaObj->pushExternalDeclIntoScope(PreloadedDecls[I], 
+                                       PreloadedDecls[I]->getDeclName());
   }
   PreloadedDecls.clear();
 
@@ -4446,7 +4431,10 @@ void ASTReader::InitializeSema(Sema &S) {
 IdentifierInfo* ASTReader::get(const char *NameStart, const char *NameEnd) {
   IdentifierLookupVisitor Visitor(StringRef(NameStart, NameEnd - NameStart));
   ModuleMgr.visit(IdentifierLookupVisitor::visit, &Visitor);
-  return Visitor.getIdentifierInfo();
+  IdentifierInfo *II = Visitor.getIdentifierInfo();
+  if (II)
+    II->setOutOfDate(false);
+  return II;
 }
 
 namespace clang {
@@ -4775,13 +4763,10 @@ ASTReader::SetGloballyVisibleDecls(IdentifierInfo *II,
   for (unsigned I = 0, N = DeclIDs.size(); I != N; ++I) {
     NamedDecl *D = cast<NamedDecl>(GetDecl(DeclIDs[I]));
     if (SemaObj) {
-      if (SemaObj->TUScope) {
-        // Introduce this declaration into the translation-unit scope
-        // and add it to the declaration chain for this identifier, so
-        // that (unqualified) name lookup will find it.
-        SemaObj->TUScope->AddDecl(D);
-      }
-      SemaObj->IdResolver.AddDeclToIdentifierChain(II, D);
+      // Introduce this declaration into the translation-unit scope
+      // and add it to the declaration chain for this identifier, so
+      // that (unqualified) name lookup will find it.
+      SemaObj->pushExternalDeclIntoScope(D, II);
     } else {
       // Queue this declaration so that it will be added to the
       // translation unit scope and identifier's declaration chain

lib/Serialization/ASTWriter.cpp

@@ -1664,8 +1664,9 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
     // chained PCH, by storing the offset into the original PCH rather than
     // writing the macro definition a second time.
     if (MI->isBuiltinMacro() ||
-        (Chain && Name->isFromAST() && MI->isFromAST() &&
-        !MI->hasChangedAfterLoad()))
+        (Chain && 
+         Name->isFromAST() && !Name->hasChangedSinceDeserialization() && 
+         MI->isFromAST() && !MI->hasChangedAfterLoad()))
       continue;
 
     AddIdentifierRef(Name, Record);
@@ -2191,6 +2192,7 @@ namespace {
 class ASTIdentifierTableTrait {
   ASTWriter &Writer;
   Preprocessor &PP;
+  IdentifierResolver &IdResolver;
   bool IsModule;
   
   /// \brief Determines whether this is an "interesting" identifier
@@ -2200,6 +2202,7 @@ class ASTIdentifierTableTrait {
     if (II->isPoisoned() ||
         II->isExtensionToken() ||
         II->getObjCOrBuiltinID() ||
+        II->hasRevertedTokenIDToIdentifier() ||
         II->getFETokenInfo<void>())
       return true;
 
@@ -2223,15 +2226,16 @@ public:
   typedef IdentID data_type;
   typedef data_type data_type_ref;
 
-  ASTIdentifierTableTrait(ASTWriter &Writer, Preprocessor &PP, bool IsModule)
-    : Writer(Writer), PP(PP), IsModule(IsModule) { }
+  ASTIdentifierTableTrait(ASTWriter &Writer, Preprocessor &PP, 
+                          IdentifierResolver &IdResolver, bool IsModule)
+    : Writer(Writer), PP(PP), IdResolver(IdResolver), IsModule(IsModule) { }
 
   static unsigned ComputeHash(const IdentifierInfo* II) {
     return llvm::HashString(II->getName());
   }
 
   std::pair<unsigned,unsigned>
-    EmitKeyDataLength(raw_ostream& Out, IdentifierInfo* II, IdentID ID) {
+  EmitKeyDataLength(raw_ostream& Out, IdentifierInfo* II, IdentID ID) {
     unsigned KeyLen = II->getLength() + 1;
     unsigned DataLen = 4; // 4 bytes for the persistent ID << 1
     MacroInfo *Macro = 0;
@@ -2239,8 +2243,9 @@ public:
       DataLen += 2; // 2 bytes for builtin ID, flags
       if (hasMacroDefinition(II, Macro))
         DataLen += 4;
-      for (IdentifierResolver::iterator D = IdentifierResolver::begin(II),
-                                     DEnd = IdentifierResolver::end();
+      
+      for (IdentifierResolver::iterator D = IdResolver.begin(II),
+                                     DEnd = IdResolver.end();
            D != DEnd; ++D)
         DataLen += sizeof(DeclID);
     }
@@ -2285,14 +2290,13 @@ public:
     // Emit the declaration IDs in reverse order, because the
     // IdentifierResolver provides the declarations as they would be
     // visible (e.g., the function "stat" would come before the struct
-    // "stat"), but IdentifierResolver::AddDeclToIdentifierChain()
-    // adds declarations to the end of the list (so we need to see the
-    // struct "status" before the function "status").
+    // "stat"), but the ASTReader adds declarations to the end of the list 
+    // (so we need to see the struct "status" before the function "status").
     // Only emit declarations that aren't from a chained PCH, though.
-    SmallVector<Decl *, 16> Decls(IdentifierResolver::begin(II),
-                                        IdentifierResolver::end());
+    SmallVector<Decl *, 16> Decls(IdResolver.begin(II),
+                                  IdResolver.end());
     for (SmallVector<Decl *, 16>::reverse_iterator D = Decls.rbegin(),
-                                                      DEnd = Decls.rend();
+                                                DEnd = Decls.rend();
          D != DEnd; ++D)
       clang::io::Emit32(Out, Writer.getDeclID(*D));
   }
@@ -2304,14 +2308,16 @@ public:
 /// The identifier table consists of a blob containing string data
 /// (the actual identifiers themselves) and a separate "offsets" index
 /// that maps identifier IDs to locations within the blob.
-void ASTWriter::WriteIdentifierTable(Preprocessor &PP, bool IsModule) {
+void ASTWriter::WriteIdentifierTable(Preprocessor &PP, 
+                                     IdentifierResolver &IdResolver,
+                                     bool IsModule) {
   using namespace llvm;
 
   // Create and write out the blob that contains the identifier
   // strings.
   {
     OnDiskChainedHashTableGenerator<ASTIdentifierTableTrait> Generator;
-    ASTIdentifierTableTrait Trait(*this, PP, IsModule);
+    ASTIdentifierTableTrait Trait(*this, PP, IdResolver, IsModule);
 
     // Look for any identifiers that were named while processing the
     // headers, but are otherwise not needed. We add these to the hash
@@ -2330,7 +2336,8 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP, bool IsModule) {
            ID = IdentifierIDs.begin(), IDEnd = IdentifierIDs.end();
          ID != IDEnd; ++ID) {
       assert(ID->first && "NULL identifier in identifier table");
-      if (!Chain || !ID->first->isFromAST())
+      if (!Chain || !ID->first->isFromAST() || 
+          ID->first->hasChangedSinceDeserialization())
         Generator.insert(const_cast<IdentifierInfo *>(ID->first), ID->second, 
                          Trait);
     }
@@ -2339,7 +2346,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP, bool IsModule) {
     llvm::SmallString<4096> IdentifierTable;
     uint32_t BucketOffset;
     {
-      ASTIdentifierTableTrait Trait(*this, PP, IsModule);
+      ASTIdentifierTableTrait Trait(*this, PP, IdResolver, IsModule);
       llvm::raw_svector_ostream Out(IdentifierTable);
       // Make sure that no bucket is at offset 0
       clang::io::Emit32(Out, 0);
@@ -2791,6 +2798,15 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
       getIdentifierRef(&Table.get(BuiltinNames[I]));
   }
 
+  // If there are any out-of-date identifiers, bring them up to date.
+  if (ExternalPreprocessorSource *ExtSource = PP.getExternalSource()) {
+    for (IdentifierTable::iterator ID = PP.getIdentifierTable().begin(),
+                                IDEnd = PP.getIdentifierTable().end();
+         ID != IDEnd; ++ID)
+      if (ID->second->isOutOfDate())
+        ExtSource->updateOutOfDateIdentifier(*ID->second);
+  }
+
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
   // headers.
@@ -3018,7 +3034,7 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
   WriteHeaderSearch(PP.getHeaderSearchInfo(), isysroot);
   WriteSelectors(SemaRef);
   WriteReferencedSelectorsPool(SemaRef);
-  WriteIdentifierTable(PP, IsModule);
+  WriteIdentifierTable(PP, SemaRef.IdResolver, IsModule);
   WriteFPPragmaOptions(SemaRef.getFPOptions());
   WriteOpenCLExtensions(SemaRef);
 

test/Modules/redeclarations.m

@@ -0,0 +1,22 @@
+#ifdef MODULE_LEFT
+@class NSObject;
+#endif
+
+#ifdef MODULE_RIGHT
+@interface NSObject
+@end
+#endif
+
+#ifdef APP
+__import_module__ Right;
+__import_module__ Left;
+
+@interface MyObject : NSObject
+@end
+#endif
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodule-cache-path %t -fdisable-module-hash -emit-module -o %t/Left.pcm -DMODULE_LEFT %s
+// RUN: %clang_cc1 -fmodule-cache-path %t -fdisable-module-hash -emit-module -o %t/Right.pcm -DMODULE_RIGHT %s
+// RUN: %clang_cc1 -fmodule-cache-path %t -fdisable-module-hash -DAPP %s -verify
+