[cfe-commits] r136708 - in /cfe/trunk: 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
Douglas Gregor
dgregor at apple.com
Tue Aug 2 11:32:54 PDT 2011
Author: dgregor
Date: Tue Aug 2 13:32:54 2011
New Revision: 136708
URL: http://llvm.org/viewvc/llvm-project?rev=136708&view=rev
Log:
Change the hashing function for DeclContext lookup within an AST file
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.
Added:
cfe/trunk/test/PCH/chain-conversion-lookup.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=136708&r1=136707&r2=136708&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Tue Aug 2 13:32:54 2011
@@ -115,18 +115,6 @@
}
};
- /// \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;
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=136708&r1=136707&r2=136708&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Aug 2 13:32:54 2011
@@ -382,24 +382,11 @@
/// \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 @@
/// 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 @@
/// \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 @@
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;
Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=136708&r1=136707&r2=136708&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Tue Aug 2 13:32:54 2011
@@ -72,6 +72,19 @@
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 @@
/// 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.
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=136708&r1=136707&r2=136708&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 2 13:32:54 2011
@@ -782,17 +782,12 @@
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 @@
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 @@
(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 @@
}
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 @@
(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 @@
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 @@
return QualType();
TypesLoaded[Index]->setFromAST();
- TypeIdxs[TypesLoaded[Index]] = TypeIdx::fromTypeID(ID);
if (DeserializationListener)
DeserializationListener->TypeRead(TypeIdx::fromTypeID(ID),
TypesLoaded[Index]);
@@ -4007,62 +3998,6 @@
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 @@
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 @@
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 @@
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);
}
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=136708&r1=136707&r2=136708&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Aug 2 13:32:54 2011
@@ -2464,7 +2464,6 @@
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 @@
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 @@
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 @@
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 @@
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();
Added: cfe/trunk/test/PCH/chain-conversion-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/chain-conversion-lookup.cpp?rev=136708&view=auto
==============================================================================
--- cfe/trunk/test/PCH/chain-conversion-lookup.cpp (added)
+++ cfe/trunk/test/PCH/chain-conversion-lookup.cpp Tue Aug 2 13:32:54 2011
@@ -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
More information about the cfe-commits
mailing list