[Modules] Cope better with top-level declarations loaded after being declared in the current translation unit <rdar://problem/13189985>.

These two related tweaks to keep the information associated with a
given identifier correct when the identifier has been given some
top-level information (say, a top-level declaration) and more
information is then loaded from a module. The first ensures that an
identifier that was "interesting" before being loaded from an AST is
considered to be different from its on-disk counterpart. Otherwise, we
lose such changes when writing the current translation unit as a
module.

Second, teach the code that injects AST-loaded names into the
identifier chain for name lookup to keep the most recent declaration,
so that we don't end up confusing our declaration chains by having a
different declaration in there.

llvm-svn: 174895

clang/lib/Sema/IdentifierResolver.cpp

@@ -302,6 +302,14 @@ static DeclMatchKind compareDeclarations(NamedDecl *Existing, NamedDecl *New) {
 
   // If the declarations are redeclarations of each other, keep the newest one.
   if (Existing->getCanonicalDecl() == New->getCanonicalDecl()) {
+    // If either of these is the most recent declaration, use it.
+    Decl *MostRecent = Existing->getMostRecentDecl();
+    if (Existing == MostRecent)
+      return DMK_Ignore;
+
+    if (New == MostRecent)
+      return DMK_Replace;
+
     // 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(), 

clang/lib/Serialization/ASTReader.cpp

@@ -457,6 +457,16 @@ ASTIdentifierLookupTraitBase::ReadKey(const unsigned char* d, unsigned n) {
   return StringRef((const char*) d, n-1);
 }
 
+/// \brief Whether the given identifier is "interesting".
+static bool isInterestingIdentifier(IdentifierInfo &II) {
+  return II.isPoisoned() ||
+         II.isExtensionToken() ||
+         II.getObjCOrBuiltinID() ||
+         II.hasRevertedTokenIDToIdentifier() ||
+         II.hadMacroDefinition() ||
+         II.getFETokenInfo<void>();
+}
+
 IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
                                                    const unsigned char* d,
                                                    unsigned DataLen) {
@@ -477,8 +487,13 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
       KnownII = II;
     }
     Reader.SetIdentifierInfo(ID, II);
-    II->setIsFromAST();
-    Reader.markIdentifierUpToDate(II);    
+    if (!II->isFromAST()) {
+      bool WasInteresting = isInterestingIdentifier(*II);
+      II->setIsFromAST();
+      if (WasInteresting)
+        II->setChangedSinceDeserialization();
+    }
+    Reader.markIdentifierUpToDate(II);
     return II;
   }
 
@@ -506,7 +521,12 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
     KnownII = II;
   }
   Reader.markIdentifierUpToDate(II);
-  II->setIsFromAST();
+  if (!II->isFromAST()) {
+    bool WasInteresting = isInterestingIdentifier(*II);
+    II->setIsFromAST();
+    if (WasInteresting)
+      II->setChangedSinceDeserialization();
+  }
 
   // Set or check the various bits in the IdentifierInfo structure.
   // Token IDs are read-only.

clang/test/Modules/Inputs/module.map

@@ -86,6 +86,10 @@ module redecl_merge_right {
   export *
 }
 module redecl_merge_bottom { 
+  explicit module prefix {
+    header "redecl-merge-bottom-prefix.h"
+  }
+
   header "redecl-merge-bottom.h" 
   export *
 }

clang/test/Modules/Inputs/redecl-merge-bottom-prefix.h

@@ -0,0 +1,4 @@
+// A class that is declared in the 'bottom' module, then loaded from
+// one of the modules it depends on. It needs to be visible when this
+// module is loaded.
+@class DeclaredThenLoaded;

clang/test/Modules/Inputs/redecl-merge-bottom.h

@@ -18,3 +18,8 @@ struct S3;
 
 void refers_to_C4(C4*);
 
+@interface UnrelatedToDeclaredThenLoaded
+- declaredThenLoadedMethod;
+@end
+
+@class DeclaredThenLoaded;

clang/test/Modules/Inputs/redecl-merge-left.h

@@ -82,3 +82,11 @@ extern double var3;
 // top level.
 typedef void funcptr_with_id(int id);
 
+// A class that is declared in the 'bottom' module, then loaded from
+// one of the modules it depends on.
+@interface DeclaredThenLoaded
+- declaredThenLoadedMethod;
+@end
+
+@class DeclaredThenLoaded;
+

clang/test/Modules/redecl-merge2.m

@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -verify -Wno-objc-root-class
+// expected-no-diagnostics
+
+@import redecl_merge_bottom.prefix;
+
+DeclaredThenLoaded *dtl;
+