]> granicus.if.org Git - clang/commitdiff
Change the hashing function for DeclContext lookup within an AST file
authorDouglas Gregor <dgregor@apple.com>
Tue, 2 Aug 2011 18:32:54 +0000 (18:32 +0000)
committerDouglas Gregor <dgregor@apple.com>
Tue, 2 Aug 2011 18:32:54 +0000 (18:32 +0000)
by eliminating the type ID from constructor, destructor, and
conversion function names. There are several reasons for this change:
  - A given type (say, int*) isn't guaranteed to have a single, unique
  type ID within a chain of PCH files. Hence, we could end up hashing
  based on the wrong type ID, causing name lookup to fail.

  - The mapping from types back to type IDs required one DenseMap
  entry for every type that was ever deserialized, which was an
  unacceptable cost to support just the name lookup of constructors,
  destructors, and conversion functions. Plus, this mapping could
  never actually work with chained or multiple PCH, based on the first
  bullet.

Once we have eliminated the type from the hash function, these
problems go away, as does my horrible "reverse type remap" hack, which
was doomed from the start (see bullet #1 above) and far too
complicated.

However, note that removing the type from the hash function means that
all constructors, destructors, and conversion functions have the same
hash key, so I've updated the caller to double-check that the
declarations found have the appropriate name.

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

include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/PCH/chain-conversion-lookup.cpp [new file with mode: 0644]

index 21fb8fa4edf644327741ff25e1586566febcc741..761de497c7307c8b8f13d1111e4ad5f2ca31bad5 100644 (file)
@@ -115,18 +115,6 @@ namespace clang {
       }
     };
 
-    /// \brief Map that provides the ID numbers of each type within the
-    /// output stream, plus those deserialized from a chained PCH.
-    ///
-    /// The ID numbers of types are consecutive (in order of discovery)
-    /// and start at 1. 0 is reserved for NULL. When types are actually
-    /// stored in the stream, the ID number is shifted by 2 bits to
-    /// allow for the const/volatile qualifiers.
-    ///
-    /// Keys in the map never have const/volatile qualifiers.
-    typedef llvm::DenseMap<QualType, TypeIdx, UnsafeQualTypeDenseMapInfo>
-        TypeIdxMap;
-
     /// \brief An ID number that refers to an identifier in an AST file.
     typedef uint32_t IdentID;
 
index 5d663105cca02fda07c249bb9312b5f098317815..09a5c31bad08a4ddb74ffa5237acc44c6be3a908 100644 (file)
@@ -382,24 +382,11 @@ public:
   
   /// \brief Base type ID for types local to this module as represented in 
   /// the global type ID space.
-  serialization::TypeID GlobalBaseTypeIndex;
+  serialization::TypeID BaseTypeIndex;
   
   /// \brief Remapping table for type IDs in this module.
   ContinuousRangeMap<uint32_t, int, 2> TypeRemap;
 
-  /// \brief Base type ID for types local to this module as represented in
-  /// the module's type ID space.
-  serialization::TypeID LocalBaseTypeIndex;
-
-  /// \brief Remapping table that maps from a type as represented as a module
-  /// and local type index to the index used within the current module to
-  /// refer to that same type.
-  /// 
-  /// This mapping is effectively the reverse of the normal \c TypeRemap, and
-  /// is used specifically by ASTReader::GetTypeIdx() to help map between
-  /// global type IDs and a module's view of the same type ID as a hash value.
-  llvm::DenseMap<Module *, int> ReverseTypeRemap;
-
   // === Miscellaneous ===
   
   /// \brief Diagnostic IDs and their mappings that the user changed.
@@ -595,17 +582,6 @@ private:
   /// global type ID to produce a local ID.
   GlobalTypeMapType GlobalTypeMap;
 
-  /// \brief Map that provides the ID numbers of each type within the
-  /// output stream, plus those deserialized from a chained PCH.
-  ///
-  /// The ID numbers of types are consecutive (in order of discovery)
-  /// and start at 1. 0 is reserved for NULL. When types are actually
-  /// stored in the stream, the ID number is shifted by 2 bits to
-  /// allow for the const/volatile qualifiers.
-  ///
-  /// Keys in the map never have const/volatile qualifiers.
-  serialization::TypeIdxMap TypeIdxs;
-
   /// \brief Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1253,10 +1229,6 @@ public:
   /// \brief Map a local type ID within a given AST file into a global type ID.
   serialization::TypeID getGlobalTypeID(Module &F, unsigned LocalID) const;
   
-  /// \brief Map a global type ID to an ID as it would be locally expressed
-  /// in the given model.
-  unsigned getLocalTypeID(Module &M, serialization::TypeID GlobalID);
-  
   /// \brief Read a type from the current position in the given record, which 
   /// was read from the given AST file.
   QualType readType(Module &F, const RecordData &Record, unsigned &Idx) {
@@ -1266,16 +1238,6 @@ public:
     return getLocalType(F, Record[Idx++]);
   }
   
-  /// \brief Returns the type ID associated with the given type.
-  /// If the type didn't come from the AST file the ID that is returned is
-  /// marked as "doesn't exist in AST".
-  serialization::TypeID GetTypeID(QualType T) const;
-
-  /// \brief Returns the type index associated with the given type.
-  /// If the type didn't come from the AST file the index that is returned is
-  /// marked as "doesn't exist in AST".
-  serialization::TypeIdx GetTypeIdx(QualType T) const;
-
   /// \brief Map from a local declaration ID within a given module to a 
   /// global declaration ID.
   serialization::DeclID getGlobalDeclID(Module &F, unsigned LocalID) const;
index 051dd2164a08643c70ee4ff882bb1c8897745aef..a9c7213934f828e533d2c117fe9132552a7c149a 100644 (file)
@@ -72,6 +72,19 @@ public:
 
   friend class ASTDeclWriter;
 private:
+  /// \brief Map that provides the ID numbers of each type within the
+  /// output stream, plus those deserialized from a chained PCH.
+  ///
+  /// The ID numbers of types are consecutive (in order of discovery)
+  /// and start at 1. 0 is reserved for NULL. When types are actually
+  /// stored in the stream, the ID number is shifted by 2 bits to
+  /// allow for the const/volatile qualifiers.
+  ///
+  /// Keys in the map never have const/volatile qualifiers.
+  typedef llvm::DenseMap<QualType, serialization::TypeIdx, 
+                         serialization::UnsafeQualTypeDenseMapInfo>
+    TypeIdxMap;
+
   /// \brief The bitstream writer used to emit this precompiled header.
   llvm::BitstreamWriter &Stream;
 
@@ -142,7 +155,7 @@ private:
   /// allow for the const/volatile qualifiers.
   ///
   /// Keys in the map never have const/volatile qualifiers.
-  serialization::TypeIdxMap TypeIdxs;
+  TypeIdxMap TypeIdxs;
 
   /// \brief Offset of each type in the bitstream, indexed by
   /// the type's ID.
index 1b351a30dc84f6b62627a55534f5542ff0b20ac7..4205cdb087e8e821d782141e8d01025d20d0b725 100644 (file)
@@ -782,17 +782,12 @@ public:
     case DeclarationName::ObjCMultiArgSelector:
       ID.AddInteger(serialization::ComputeHash(Selector(Key.Data)));
       break;
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
-      if (TypeID(Key.Data) == TypeID(-1))
-        ID.AddInteger((TypeID)Key.Data);
-      else
-        ID.AddInteger(Reader.getLocalTypeID(F, (TypeID)Key.Data));
-      break;
     case DeclarationName::CXXOperatorName:
       ID.AddInteger((OverloadedOperatorKind)Key.Data);
       break;
+    case DeclarationName::CXXConstructorName:
+    case DeclarationName::CXXDestructorName:
+    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXUsingDirective:
       break;
     }
@@ -812,18 +807,17 @@ public:
     case DeclarationName::ObjCMultiArgSelector:
       Key.Data = (uint64_t)Name.getObjCSelector().getAsOpaquePtr();
       break;
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
-      Key.Data = Reader.GetTypeID(Name.getCXXNameType());
-      break;
     case DeclarationName::CXXOperatorName:
       Key.Data = Name.getCXXOverloadedOperator();
       break;
     case DeclarationName::CXXLiteralOperatorName:
       Key.Data = (uint64_t)Name.getCXXLiteralIdentifier();
       break;
+    case DeclarationName::CXXConstructorName:
+    case DeclarationName::CXXDestructorName:
+    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXUsingDirective:
+      Key.Data = 0;
       break;
     }
 
@@ -892,18 +886,17 @@ public:
          (uint64_t)Reader.getLocalSelector(F, ReadUnalignedLE32(d))
                      .getAsOpaquePtr();
       break;
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
-      Key.Data = Reader.getGlobalTypeID(F, ReadUnalignedLE32(d)); // TypeID
-      break;
     case DeclarationName::CXXOperatorName:
       Key.Data = *d++; // OverloadedOperatorKind
       break;
     case DeclarationName::CXXLiteralOperatorName:
       Key.Data = (uint64_t)Reader.getLocalIdentifier(F, ReadUnalignedLE32(d));
       break;
+    case DeclarationName::CXXConstructorName:
+    case DeclarationName::CXXDestructorName:
+    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXUsingDirective:
+      Key.Data = 0;
       break;
     }
 
@@ -2059,16 +2052,16 @@ ASTReader::ReadASTBlock(Module &F) {
       }
       F.TypeOffsets = (const uint32_t *)BlobStart;
       F.LocalNumTypes = Record[0];
-      F.LocalBaseTypeIndex = Record[1];
-      F.GlobalBaseTypeIndex = getTotalNumTypes();
+      unsigned LocalBaseTypeIndex = Record[1];
+      F.BaseTypeIndex = getTotalNumTypes();
         
       if (F.LocalNumTypes > 0) {
         // Introduce the global -> local mapping for types within this module.
         GlobalTypeMap.insert(std::make_pair(getTotalNumTypes(), &F));
         
         // Introduce the local -> global mapping for types within this module.
-        F.TypeRemap.insert(std::make_pair(F.LocalBaseTypeIndex, 
-                             F.GlobalBaseTypeIndex - F.LocalBaseTypeIndex));
+        F.TypeRemap.insert(std::make_pair(LocalBaseTypeIndex, 
+                             F.BaseTypeIndex - LocalBaseTypeIndex));
         
         TypesLoaded.resize(TypesLoaded.size() + F.LocalNumTypes);
       }
@@ -2323,8 +2316,7 @@ ASTReader::ReadASTBlock(Module &F) {
         (void)CXXBaseSpecifiersIDOffset;
         
         TypeRemap.insert(std::make_pair(TypeIndexOffset, 
-                                    OM->GlobalBaseTypeIndex - TypeIndexOffset));
-        F.ReverseTypeRemap[OM] = TypeIndexOffset - OM->GlobalBaseTypeIndex;
+                                    OM->BaseTypeIndex - TypeIndexOffset));
       }
       break;
     }
@@ -3248,7 +3240,7 @@ ASTReader::RecordLocation ASTReader::TypeCursorForIndex(unsigned Index) {
   GlobalTypeMapType::iterator I = GlobalTypeMap.find(Index);
   assert(I != GlobalTypeMap.end() && "Corrupted global type map");
   Module *M = I->second;
-  return RecordLocation(M, M->TypeOffsets[Index - M->GlobalBaseTypeIndex]);
+  return RecordLocation(M, M->TypeOffsets[Index - M->BaseTypeIndex]);
 }
 
 /// \brief Read and return the type with the given index..
@@ -3978,7 +3970,6 @@ QualType ASTReader::GetType(TypeID ID) {
       return QualType();
 
     TypesLoaded[Index]->setFromAST();
-    TypeIdxs[TypesLoaded[Index]] = TypeIdx::fromTypeID(ID);
     if (DeserializationListener)
       DeserializationListener->TypeRead(TypeIdx::fromTypeID(ID),
                                         TypesLoaded[Index]);
@@ -4007,62 +3998,6 @@ ASTReader::getGlobalTypeID(Module &F, unsigned LocalID) const {
   return (GlobalIndex << Qualifiers::FastWidth) | FastQuals;
 }
 
-unsigned ASTReader::getLocalTypeID(Module &M, serialization::TypeID GlobalID) {
-  unsigned FastQuals = GlobalID & Qualifiers::FastMask;
-  unsigned GlobalIndex = GlobalID >> Qualifiers::FastWidth;
-
-  if (GlobalIndex < NUM_PREDEF_TYPE_IDS)
-    return GlobalID;
-  
-  GlobalIndex -= NUM_PREDEF_TYPE_IDS;
-  RecordLocation Loc = TypeCursorForIndex(GlobalIndex);
-  
-  if (Loc.F == &M) {
-    // Simple case: the type ID came from the module we're asked to provide a
-    // type ID for. Shift the index appropriately;
-    unsigned LocalIndex 
-      = GlobalIndex - M.GlobalBaseTypeIndex + M.LocalBaseTypeIndex 
-      + NUM_PREDEF_TYPE_IDS ;
-    return (LocalIndex << Qualifiers::FastWidth) | FastQuals;
-  }
-
-  // Complex case: the type ID came from a module that M depends on, which may
-  // have had some remapping between the IDs used to store it in M and its
-  // location in the global space.
-  llvm::DenseMap<Module *, int>::iterator R = Loc.F->ReverseTypeRemap.find(&M);
-  if (R == Loc.F->ReverseTypeRemap.end())
-    return TypeID(-1); // FIXME: This is a terrible failure case
-  
-  unsigned LocalIndex = GlobalIndex - Loc.F->GlobalBaseTypeIndex 
-                      + R->second + NUM_PREDEF_TYPE_IDS;
-  return (LocalIndex << Qualifiers::FastWidth) | FastQuals;
-}
-
-TypeID ASTReader::GetTypeID(QualType T) const {
-  return MakeTypeID(T,
-              std::bind1st(std::mem_fun(&ASTReader::GetTypeIdx), this));
-}
-
-TypeIdx ASTReader::GetTypeIdx(QualType T) const {
-  if (T.isNull())
-    return TypeIdx();
-  assert(!T.getLocalFastQualifiers());
-
-  // FIXME: Modules can't handle this. It's even dubious with chained PCH,
-  // because the same type (say, int*) can be serialized into different
-  // PCH files within the chain, and there's no way to know which of the
-  // ID numbers we actually want.
-  TypeIdxMap::const_iterator I = TypeIdxs.find(T);
-  // GetTypeIdx is mostly used for computing the hash of DeclarationNames and
-  // comparing keys of ASTDeclContextNameLookupTable.
-  // If the type didn't come from the AST file use a specially marked index
-  // so that any hash/key comparison fail since no such index is stored
-  // in a AST file.
-  if (I == TypeIdxs.end())
-    return TypeIdx(-1);
-  return I->second;
-}
-
 TemplateArgumentLocInfo
 ASTReader::GetTemplateArgumentLocInfo(Module &F,
                                       TemplateArgument::ArgKind Kind,
@@ -4271,8 +4206,25 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
       continue;
 
     ASTDeclContextNameLookupTrait::data_type Data = *Pos;
-    for (; Data.first != Data.second; ++Data.first)
-      Decls.push_back(GetLocalDeclAs<NamedDecl>(*I->F, *Data.first));
+    for (; Data.first != Data.second; ++Data.first) {
+      NamedDecl *ND = GetLocalDeclAs<NamedDecl>(*I->F, *Data.first);
+      if (!ND)
+        continue;
+      
+      if (ND->getDeclName() != Name) {
+        assert(!Name.getCXXNameType().isNull() && 
+               "Name mismatch without a type");
+        continue;
+      }
+      
+      Decls.push_back(ND);
+    }
+    
+    // If we rejected all of the declarations we found, e.g., because the
+    // name didn't actually match, continue looking through DeclContexts.
+    if (Decls.empty())
+      continue;
+    
     break;
   }
 
@@ -5532,8 +5484,7 @@ Module::Module(ModuleKind Kind)
     DeclOffsets(0), BaseDeclID(0),
     LocalNumCXXBaseSpecifiers(0), CXXBaseSpecifiersOffsets(0),
     BaseCXXBaseSpecifiersID(0),
-    LocalNumTypes(0), TypeOffsets(0), GlobalBaseTypeIndex(0), 
-    LocalBaseTypeIndex(0), StatCache(0),
+    LocalNumTypes(0), TypeOffsets(0), BaseTypeIndex(0), StatCache(0),
     NumPreallocatedPreprocessingEntities(0)
 {}
 
@@ -5575,7 +5526,7 @@ void Module::dump() {
   llvm::errs() << "  Base source location offset: " << SLocEntryBaseOffset 
                << '\n';
   dumpLocalRemap("Source location offset map", SLocRemap);
-  llvm::errs() << "  Base type ID: " << GlobalBaseTypeIndex << '\n'
+  llvm::errs() << "  Base type ID: " << BaseTypeIndex << '\n'
                << "  Number of types: " << LocalNumTypes << '\n';
   dumpLocalRemap("Type ID map", TypeRemap);
 }
index 197690171d551184bc58325f0e4ceea89c58e4c8..66c7d5f669adb7b07dec867c7d2985c76d2ba9f5 100644 (file)
@@ -2464,7 +2464,6 @@ public:
     case DeclarationName::CXXConstructorName:
     case DeclarationName::CXXDestructorName:
     case DeclarationName::CXXConversionFunctionName:
-      ID.AddInteger(Writer.GetOrCreateTypeID(Name.getCXXNameType()));
       break;
     case DeclarationName::CXXOperatorName:
       ID.AddInteger(Name.getCXXOverloadedOperator());
@@ -2487,15 +2486,15 @@ public:
     case DeclarationName::ObjCZeroArgSelector:
     case DeclarationName::ObjCOneArgSelector:
     case DeclarationName::ObjCMultiArgSelector:
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXLiteralOperatorName:
       KeyLen += 4;
       break;
     case DeclarationName::CXXOperatorName:
       KeyLen += 1;
       break;
+    case DeclarationName::CXXConstructorName:
+    case DeclarationName::CXXDestructorName:
+    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXUsingDirective:
       break;
     }
@@ -2522,11 +2521,6 @@ public:
     case DeclarationName::ObjCMultiArgSelector:
       Emit32(Out, Writer.getSelectorRef(Name.getObjCSelector()));
       break;
-    case DeclarationName::CXXConstructorName:
-    case DeclarationName::CXXDestructorName:
-    case DeclarationName::CXXConversionFunctionName:
-      Emit32(Out, Writer.getTypeID(Name.getCXXNameType()));
-      break;
     case DeclarationName::CXXOperatorName:
       assert(Name.getCXXOverloadedOperator() < 0x100 && "Invalid operator ?");
       Emit8(Out, Name.getCXXOverloadedOperator());
@@ -2534,6 +2528,9 @@ public:
     case DeclarationName::CXXLiteralOperatorName:
       Emit32(Out, Writer.getIdentifierRef(Name.getCXXLiteralIdentifier()));
       break;
+    case DeclarationName::CXXConstructorName:
+    case DeclarationName::CXXDestructorName:
+    case DeclarationName::CXXConversionFunctionName:
     case DeclarationName::CXXUsingDirective:
       break;
     }
@@ -3074,7 +3071,7 @@ void ASTWriter::WriteASTChain(Sema &SemaRef, MemorizeStatCalls *StatCalls,
       io::Emit32(Out, (*M)->BaseSelectorID);
       io::Emit32(Out, (*M)->BaseDeclID);
       io::Emit32(Out, (*M)->BaseCXXBaseSpecifiersID);
-      io::Emit32(Out, (*M)->GlobalBaseTypeIndex);
+      io::Emit32(Out, (*M)->BaseTypeIndex);
     }
   }
   Record.clear();
diff --git a/test/PCH/chain-conversion-lookup.cpp b/test/PCH/chain-conversion-lookup.cpp
new file mode 100644 (file)
index 0000000..db9d8fc
--- /dev/null
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -chain-include %s -chain-include %s
+
+#if !defined(PASS1)
+#define PASS1
+struct X {
+  operator int*();
+};
+
+struct Z {
+  operator int*();
+};
+#elif !defined(PASS2)
+#define PASS2
+struct Y {
+  operator int *();
+};
+#else
+int main() {
+  X x;
+  int *ip = x.operator int*();
+  Y y;
+  int *ip2 = y.operator int*();
+  Z z;
+  int *ip3 = z.operator int*();
+}
+#endif