[cfe-commits] r147123 - in /cfe/trunk: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderDecl.cpp test/Modules/Inputs/redecl-merge-left.h test/Modules/Inputs/redecl-merge-right.h test/Modules/redecl-merge.m

Douglas Gregor dgregor at apple.com
Wed Dec 21 17:48:48 PST 2011


Author: dgregor
Date: Wed Dec 21 19:48:48 2011
New Revision: 147123

URL: http://llvm.org/viewvc/llvm-project?rev=147123&view=rev
Log:
When deserializing an Objective-C class, check whether we have another
declaration of that same class that either came from some other module
or occurred in the translation unit loading the module. In this case,
we need to merge the two redeclaration chains immediately so that all
such declarations have the same canonical declaration in the resulting
AST (even though they don't in the module files we've imported).

Focusing on Objective-C classes until I'm happy with the design, then
I'll both (1) extend this notion to other kinds of declarations, and
(2) optimize away this extra checking when we're not dealing with
modules. For now, doing this checking for PCH files/preambles gives us
better testing coverage.


Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/test/Modules/Inputs/redecl-merge-left.h
    cfe/trunk/test/Modules/Inputs/redecl-merge-right.h
    cfe/trunk/test/Modules/redecl-merge.m

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Dec 21 19:48:48 2011
@@ -34,6 +34,8 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/OwningPtr.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/DenseSet.h"
@@ -673,6 +675,14 @@
   /// \brief Keeps track of the elements added to PendingDeclChains.
   llvm::SmallSet<serialization::DeclID, 16> PendingDeclChainsKnown;
 
+  typedef llvm::DenseMap<Decl *, llvm::SmallVector<serialization::DeclID, 2> >
+    MergedDeclsMap;
+    
+  /// \brief A mapping from canonical declarations to the set of additional
+  /// (global, previously-canonical) declaration IDs that have been merged with
+  /// that canonical declaration.
+  MergedDeclsMap MergedDecls;
+  
   /// \brief We delay loading the chain of objc categories after recursive
   /// loading of declarations is finished.
   std::vector<std::pair<ObjCInterfaceDecl *, serialization::DeclID> >

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Dec 21 19:48:48 2011
@@ -6083,7 +6083,7 @@
       loadPendingDeclChain(PendingDeclChains[I]);
     }
     PendingDeclChains.clear();
-
+    
     for (std::vector<std::pair<ObjCInterfaceDecl *,
                                serialization::DeclID> >::iterator
            I = PendingChainedObjCCategories.begin(),

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Dec 21 19:48:48 2011
@@ -14,6 +14,8 @@
 
 #include "ASTCommon.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Sema/IdentifierResolver.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
@@ -104,6 +106,82 @@
     void InitializeCXXDefinitionData(CXXRecordDecl *D,
                                      CXXRecordDecl *DefinitionDecl,
                                      const RecordData &Record, unsigned &Idx);
+    
+    /// \brief RAII class used to capture the first ID within a redeclaration
+    /// chain and to introduce it into the list of pending redeclaration chains
+    /// on destruction.
+    ///
+    /// The caller can choose not to introduce this ID into the redeclaration
+    /// chain by calling \c suppress().
+    class RedeclarableResult {
+      ASTReader &Reader;
+      GlobalDeclID FirstID;
+      mutable bool Owning;
+      
+      RedeclarableResult &operator=(RedeclarableResult&); // DO NOT IMPLEMENT
+      
+    public:
+      RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID)
+        : Reader(Reader), FirstID(FirstID), Owning(true) { }
+
+      RedeclarableResult(const RedeclarableResult &Other)
+        : Reader(Other.Reader), FirstID(Other.FirstID), Owning(Other.Owning) 
+      { 
+        Other.Owning = false;
+      }
+
+      ~RedeclarableResult() {
+        if (FirstID && Owning && Reader.PendingDeclChainsKnown.insert(FirstID))
+          Reader.PendingDeclChains.push_back(FirstID);
+      }
+      
+      /// \brief Retrieve the first ID.
+      GlobalDeclID getFirstID() const { return FirstID; }
+      
+      /// \brief Do not introduce this declaration ID into the set of pending
+      /// declaration chains.
+      void suppress() {
+        Owning = false;
+      }
+    };
+    
+    /// \brief Class used to capture the result of searching for an existing
+    /// declaration of a specific kind and name, along with the ability
+    /// to update the place where this result was found (the declaration
+    /// chain hanging off an identifier or the DeclContext we searched in)
+    /// if requested.
+    class FindExistingResult {
+      ASTReader &Reader;
+      NamedDecl *New;
+      NamedDecl *Existing;
+      mutable bool AddResult;
+      
+      FindExistingResult &operator=(FindExistingResult&); // DO NOT IMPLEMENT
+      
+    public:
+      FindExistingResult(ASTReader &Reader)
+        : Reader(Reader), New(0), Existing(0), AddResult(false) { }
+      
+      FindExistingResult(ASTReader &Reader, NamedDecl *New, NamedDecl *Existing)
+        : Reader(Reader), New(New), Existing(Existing), AddResult(true) { }
+      
+      FindExistingResult(const FindExistingResult &Other)
+        : Reader(Other.Reader), New(Other.New), Existing(Other.Existing), 
+          AddResult(Other.AddResult)
+      {
+        Other.AddResult = false;
+      }
+      
+      ~FindExistingResult();
+      
+      operator NamedDecl*() const { return Existing; }
+      
+      template<typename T>
+      operator T*() const { return dyn_cast_or_null<T>(Existing); }
+    };
+    
+    FindExistingResult findExisting(NamedDecl *D);
+    
   public:
     ASTDeclReader(ASTReader &Reader, ModuleFile &F,
                   llvm::BitstreamCursor &Cursor, DeclID thisDeclID,
@@ -182,7 +260,9 @@
     void VisitBlockDecl(BlockDecl *BD);
 
     std::pair<uint64_t, uint64_t> VisitDeclContext(DeclContext *DC);
-    template <typename T> void VisitRedeclarable(Redeclarable<T> *D);
+    
+    template <typename T> 
+    RedeclarableResult VisitRedeclarable(Redeclarable<T> *D);
 
     // FIXME: Reorder according to DeclNodes.td?
     void VisitObjCMethodDecl(ObjCMethodDecl *D);
@@ -563,10 +643,31 @@
 }
 
 void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
-  VisitRedeclarable(ID);
+  RedeclarableResult Redecl = VisitRedeclarable(ID);
   VisitObjCContainerDecl(ID);
   TypeIDForTypeDecl = Reader.getGlobalTypeID(F, Record[Idx++]);
   
+  // Determine whether we need to merge this declaration with another @interface
+  // with the same name.
+  // FIXME: Not needed unless the module file graph is a DAG.
+  if (FindExistingResult ExistingRes = findExisting(ID)) {
+    if (ObjCInterfaceDecl *Existing = ExistingRes) {
+      ObjCInterfaceDecl *ExistingCanon = Existing->getCanonicalDecl();
+      ObjCInterfaceDecl *IDCanon = ID->getCanonicalDecl();
+      if (ExistingCanon != IDCanon) {
+        // Have our redeclaration link point back at the canonical declaration
+        // of the existing declaration, so that this declaration has the 
+        // appropriate canonical declaration.
+        ID->RedeclLink = ObjCInterfaceDecl::PreviousDeclLink(ExistingCanon);
+        
+        // If this declaration was the canonical declaration, make a note of 
+        // that.
+        if (IDCanon == ID)
+          Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());
+      }
+    }
+  }
+  
   ObjCInterfaceDecl *Def = ReadDeclAs<ObjCInterfaceDecl>(Record, Idx);
   if (ID == Def) {
     // Read the definition.
@@ -1143,9 +1244,9 @@
       Common->Latest = D;
       D->CommonOrPrev = Common;
     }
-    
+
     if (RedeclarableTemplateDecl *RTD
-        = ReadDeclAs<RedeclarableTemplateDecl>(Record, Idx)) {
+          = ReadDeclAs<RedeclarableTemplateDecl>(Record, Idx)) {
       assert(RTD->getKind() == D->getKind() &&
              "InstantiatedFromMemberTemplate kind mismatch");
       D->setInstantiatedFromMemberTemplateImpl(RTD);
@@ -1380,7 +1481,8 @@
 }
 
 template <typename T>
-void ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
+ASTDeclReader::RedeclarableResult 
+ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
   enum RedeclKind { FirstDeclaration = 0, FirstInFile, PointsToPrevious };
   RedeclKind Kind = (RedeclKind)Record[Idx++];
   
@@ -1415,9 +1517,9 @@
   }
   }
 
-  // Note that we need to load the other declaration chains for this ID.
-  if (Reader.PendingDeclChainsKnown.insert(FirstDeclID))
-    Reader.PendingDeclChains.push_back(FirstDeclID);
+  // The result structure takes care of note that we need to load the 
+  // other declaration chains for this ID.
+  return RedeclarableResult(Reader, FirstDeclID);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1509,6 +1611,62 @@
   return LocalOffset + M.GlobalBitOffset;
 }
 
+/// \brief Determine whether the two declarations refer to the same entity.
+static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
+  assert(X->getDeclName() == Y->getDeclName() && "Declaration name mismatch!");
+  
+  if (X == Y)
+    return true;
+  
+  // Must have the same kind.
+  if (X->getKind() != Y->getKind())
+    return false;
+  
+  // Must be in the same context.
+  if (!X->getDeclContext()->getRedeclContext()->Equals(
+         Y->getDeclContext()->getRedeclContext()))
+    return false;
+  
+  // Objective-C classes with the same name always match.
+  if (isa<ObjCInterfaceDecl>(X))
+    return true;
+  
+  // FIXME: Many other cases to implement.
+  return false;
+}
+
+ASTDeclReader::FindExistingResult::~FindExistingResult() {
+  if (!AddResult)
+    return;
+  
+  DeclContext *DC = New->getDeclContext()->getRedeclContext();
+  if (DC->isTranslationUnit() && Reader.SemaObj) {
+    if (!Existing) {
+      Reader.SemaObj->IdResolver.tryAddTopLevelDecl(New, New->getDeclName());
+    }
+  }
+}
+
+ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) {
+  DeclContext *DC = D->getDeclContext()->getRedeclContext();
+  if (!DC->isFileContext())
+    return FindExistingResult(Reader);
+  
+  if (DC->isTranslationUnit() && Reader.SemaObj) {
+    IdentifierResolver &IdResolver = Reader.SemaObj->IdResolver;
+    for (IdentifierResolver::iterator I = IdResolver.begin(D->getDeclName()), 
+                                   IEnd = IdResolver.end();
+         I != IEnd; ++I) {
+      if (isSameEntity(*I, D))
+        return FindExistingResult(Reader, D, *I);
+    }
+  }
+
+  // FIXME: Search in the DeclContext.
+  
+  return FindExistingResult(Reader, D, /*Existing=*/0);
+}
+
 void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *previous) {
   assert(D && previous);
   if (TagDecl *TD = dyn_cast<TagDecl>(D)) {
@@ -1902,14 +2060,18 @@
     }
   };
   
+  /// \brief Module visitor class that finds all of the redeclarations of a 
+  /// 
   class RedeclChainVisitor {
     ASTReader &Reader;
-    DeclID GlobalFirstID;
+    SmallVectorImpl<DeclID> &SearchDecls;
+    GlobalDeclID CanonID;
     llvm::SmallVector<std::pair<Decl *, Decl *>, 4> Chains;
     
   public:
-    RedeclChainVisitor(ASTReader &Reader, DeclID GlobalFirstID)
-      : Reader(Reader), GlobalFirstID(GlobalFirstID) { }
+    RedeclChainVisitor(ASTReader &Reader, SmallVectorImpl<DeclID> &SearchDecls,
+                       GlobalDeclID CanonID)
+      : Reader(Reader), SearchDecls(SearchDecls), CanonID(CanonID) { }
     
     static bool visit(ModuleFile &M, bool Preorder, void *UserData) {
       if (Preorder)
@@ -1918,31 +2080,49 @@
       return static_cast<RedeclChainVisitor *>(UserData)->visit(M);
     }
     
-    bool visit(ModuleFile &M) {
+    void searchForID(ModuleFile &M, GlobalDeclID GlobalID) {
       // Map global ID of the first declaration down to the local ID
       // used in this module file.
-      DeclID FirstID = Reader.mapGlobalIDToModuleFileGlobalID(M, GlobalFirstID);
-      if (!FirstID)
-        return false;
+      DeclID ID = Reader.mapGlobalIDToModuleFileGlobalID(M, GlobalID);
+      if (!ID)
+        return;
       
       // Perform a binary search to find the local redeclarations for this
       // declaration (if any).
       const LocalRedeclarationsInfo *Result
         = std::lower_bound(M.RedeclarationsInfo,
                            M.RedeclarationsInfo + M.LocalNumRedeclarationsInfos, 
-                           FirstID, CompareLocalRedeclarationsInfoToID());
+                           ID, CompareLocalRedeclarationsInfoToID());
       if (Result == M.RedeclarationsInfo + M.LocalNumRedeclarationsInfos ||
-          Result->FirstID != FirstID)
-        return false;
-
+          Result->FirstID != ID) {
+        // If we have a previously-canonical singleton declaration that was 
+        // merged into another redeclaration chain, create a trivial chain
+        // for this single declaration so that it will get wired into the 
+        // complete redeclaration chain.
+        if (GlobalID != CanonID && 
+            GlobalID - NUM_PREDEF_DECL_IDS >= M.BaseDeclID && 
+            GlobalID - NUM_PREDEF_DECL_IDS < M.BaseDeclID + M.LocalNumDecls) {
+          if (Decl *D = Reader.GetDecl(GlobalID))
+            Chains.push_back(std::make_pair(D, D));
+        }
+        
+        return;
+      }
+      
       // Dig out the starting/ending declarations.
       Decl *FirstLocalDecl = Reader.GetLocalDecl(M, Result->FirstLocalID);
       Decl *LastLocalDecl = Reader.GetLocalDecl(M, Result->LastLocalID);
       if (!FirstLocalDecl || !LastLocalDecl)
-        return false;
+        return;
       
       // Append this redeclaration chain to the list.
       Chains.push_back(std::make_pair(FirstLocalDecl, LastLocalDecl));
+    }
+    
+    bool visit(ModuleFile &M) {
+      // Visit each of the declarations.
+      for (unsigned I = 0, N = SearchDecls.size(); I != N; ++I)
+        searchForID(M, SearchDecls[I]);
       return false;
     }
     
@@ -1989,30 +2169,44 @@
 }
 
 void ASTReader::loadPendingDeclChain(serialization::GlobalDeclID ID) {
+  Decl *D = GetDecl(ID);  
+  Decl *CanonDecl = D->getCanonicalDecl();
+    
+  // Determine the set of declaration IDs we'll be searching for.
+  llvm::SmallVector<DeclID, 1> SearchDecls;
+  GlobalDeclID CanonID = 0;
+  if (D == CanonDecl) {
+    SearchDecls.push_back(ID); // Always first.
+    CanonID = ID;
+  }
+  MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl);
+  if (MergedPos != MergedDecls.end())
+    SearchDecls.append(MergedPos->second.begin(), MergedPos->second.end());  
+  
   // Build up the list of redeclaration chains.
-  RedeclChainVisitor Visitor(*this, ID);
+  RedeclChainVisitor Visitor(*this, SearchDecls, CanonID);
   ModuleMgr.visitDepthFirst(&RedeclChainVisitor::visit, &Visitor);
   
   // Retrieve the chains.
   ArrayRef<std::pair<Decl *, Decl *> > Chains = Visitor.getChains();
   if (Chains.empty())
     return;
-  
-  // FIXME: Splice local (not from AST file) declarations into the list,
-  // rather than always re-ordering them.
-  Decl *CanonDecl = GetDecl(ID);  
-  
+    
+    
   // Capture all of the parsed declarations and put them at the end.
   Decl *MostRecent = getMostRecentDecl(CanonDecl);
   Decl *FirstParsed = MostRecent;
   if (CanonDecl != MostRecent && !MostRecent->isFromASTFile()) {
     Decl *Current = MostRecent;
     while (Decl *Prev = getPreviousDecl(Current)) {
+      if (Prev == CanonDecl)
+        break;
+      
       if (Prev->isFromASTFile()) {
         Current = Prev;
         continue;
       }
-      
+            
       // Chain all of the parsed declarations together.
       ASTDeclReader::attachPreviousDecl(FirstParsed, Prev);
       FirstParsed = Prev;
@@ -2021,9 +2215,11 @@
     
     Visitor.addParsed(FirstParsed, MostRecent);
   }
-  
+
   // Hook up the separate chains.
   Chains = Visitor.getChains();
+  if (Chains[0].first != CanonDecl)
+    ASTDeclReader::attachPreviousDecl(Chains[0].first, CanonDecl);
   for (unsigned I = 1, N = Chains.size(); I != N; ++I)
     ASTDeclReader::attachPreviousDecl(Chains[I].first, Chains[I-1].second);    
   ASTDeclReader::attachLatestDecl(CanonDecl, Chains.back().second);

Modified: cfe/trunk/test/Modules/Inputs/redecl-merge-left.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/redecl-merge-left.h?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/redecl-merge-left.h (original)
+++ cfe/trunk/test/Modules/Inputs/redecl-merge-left.h Wed Dec 21 19:48:48 2011
@@ -10,6 +10,18 @@
 
 @class A;
 
+// Test declarations in different modules with no common initial
+// declaration.
+ at class C;
+void accept_a_C(C*);
+
+ at class C2;
+void accept_a_C2(C2*);
+
+ at class C3;
+void accept_a_C3(C3*);
+ at class C3;
+
 @class Explicit;
 
 int *explicit_func(void);

Modified: cfe/trunk/test/Modules/Inputs/redecl-merge-right.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/redecl-merge-right.h?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/redecl-merge-right.h (original)
+++ cfe/trunk/test/Modules/Inputs/redecl-merge-right.h Wed Dec 21 19:48:48 2011
@@ -9,6 +9,15 @@
 
 @class B;
 
+// Test declarations in different modules with no common initial
+// declaration.
+ at class C;
+C *get_a_C(void);
+ at class C2;
+C2 *get_a_C2(void);
+ at class C3;
+C3 *get_a_C3(void);
+
 @class Explicit;
 
 int *explicit_func(void);

Modified: cfe/trunk/test/Modules/redecl-merge.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/redecl-merge.m?rev=147123&r1=147122&r2=147123&view=diff
==============================================================================
--- cfe/trunk/test/Modules/redecl-merge.m (original)
+++ cfe/trunk/test/Modules/redecl-merge.m Wed Dec 21 19:48:48 2011
@@ -1,7 +1,13 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodule-cache-path %t -I %S/Inputs %s -verify
 // RUN: %clang_cc1 -x objective-c++ -fmodule-cache-path %t -I %S/Inputs %s -verify
+ at class C2;
+ at class C3;
+ at class C3;
 __import_module__ redecl_merge_left;
+
+ at class C3;
+ at class C3;
 __import_module__ redecl_merge_right;
 
 @implementation A
@@ -32,6 +38,23 @@
   struct explicit_struct es = { 0 };
 }
 
+// Test resolution of declarations from multiple modules with no
+// common original declaration.
+void test_C(C *c) {
+  c = get_a_C();
+  accept_a_C(c);
+}
+
+void test_C2(C2 *c2) {
+  c2 = get_a_C2();
+  accept_a_C2(c2);
+}
+
+void test_C3(C3 *c3) {
+  c3 = get_a_C3();
+  accept_a_C3(c3);
+}
+
 __import_module__ redecl_merge_bottom;
 
 @implementation B
@@ -48,3 +71,4 @@
   vec_int.push_back(0);
 }
 #endif
+





More information about the cfe-commits mailing list