Author: rsmith Date: Wed Aug 26 18:55:49 2015 New Revision: 246124 URL: http://llvm.org/viewvc/llvm-project?rev=246124&view=rev Log: [modules] The key to a DeclContext name lookup table is not actually a DeclarationName (because all ctor names are considered the same, and so on). Reflect this in the type used as the lookup table key. As a side-effect, remove one copy of the duplicated code used to compute the hash of the key.
Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderInternals.h 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=246124&r1=246123&r2=246124&view=diff ============================================================================== --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original) +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Wed Aug 26 18:55:49 2015 @@ -17,6 +17,7 @@ #ifndef LLVM_CLANG_SERIALIZATION_ASTBITCODES_H #define LLVM_CLANG_SERIALIZATION_ASTBITCODES_H +#include "clang/AST/DeclarationName.h" #include "clang/AST/Type.h" #include "llvm/ADT/DenseMap.h" #include "llvm/Bitcode/BitCodes.h" @@ -1480,6 +1481,51 @@ namespace clang { } }; + /// \brief A key used when looking up entities by \ref DeclarationName. + /// + /// Different \ref DeclarationNames are mapped to different keys, but the + /// same key can occasionally represent multiple names (for names that + /// contain types, in particular). + class DeclarationNameKey { + typedef unsigned NameKind; + + NameKind Kind; + uint64_t Data; + + public: + DeclarationNameKey() : Kind(), Data() {} + DeclarationNameKey(DeclarationName Name); + + DeclarationNameKey(NameKind Kind, uint64_t Data) + : Kind(Kind), Data(Data) {} + + NameKind getKind() const { return Kind; } + + IdentifierInfo *getIdentifier() const { + assert(Kind == DeclarationName::Identifier || + Kind == DeclarationName::CXXLiteralOperatorName); + return (IdentifierInfo *)Data; + } + Selector getSelector() const { + assert(Kind == DeclarationName::ObjCZeroArgSelector || + Kind == DeclarationName::ObjCOneArgSelector || + Kind == DeclarationName::ObjCMultiArgSelector); + return Selector(Data); + } + OverloadedOperatorKind getOperatorKind() const { + assert(Kind == DeclarationName::CXXOperatorName); + return (OverloadedOperatorKind)Data; + } + + /// Compute a fingerprint of this key for use in on-disk hash table. + unsigned getHash() const; + + friend bool operator==(const DeclarationNameKey &A, + const DeclarationNameKey &B) { + return A.Kind == B.Kind && A.Data == B.Data; + } + }; + /// @} } } // end namespace clang Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=246124&r1=246123&r2=246124&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Aug 26 18:55:49 2015 @@ -850,108 +850,102 @@ IdentifierInfo *ASTIdentifierLookupTrait return II; } -unsigned -ASTDeclContextNameLookupTrait::ComputeHash(const DeclNameKey &Key) { - llvm::FoldingSetNodeID ID; - ID.AddInteger(Key.Kind); - - switch (Key.Kind) { +DeclarationNameKey::DeclarationNameKey(DeclarationName Name) + : Kind(Name.getNameKind()) { + switch (Kind) { case DeclarationName::Identifier: - case DeclarationName::CXXLiteralOperatorName: - ID.AddString(((IdentifierInfo*)Key.Data)->getName()); + Data = (uint64_t)Name.getAsIdentifierInfo(); break; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - ID.AddInteger(serialization::ComputeHash(Selector(Key.Data))); + Data = (uint64_t)Name.getObjCSelector().getAsOpaquePtr(); break; case DeclarationName::CXXOperatorName: - ID.AddInteger((OverloadedOperatorKind)Key.Data); + Data = Name.getCXXOverloadedOperator(); + break; + case DeclarationName::CXXLiteralOperatorName: + Data = (uint64_t)Name.getCXXLiteralIdentifier(); break; case DeclarationName::CXXConstructorName: case DeclarationName::CXXDestructorName: case DeclarationName::CXXConversionFunctionName: case DeclarationName::CXXUsingDirective: + Data = 0; break; } - - return ID.ComputeHash(); } -ASTDeclContextNameLookupTrait::internal_key_type -ASTDeclContextNameLookupTrait::GetInternalKey( - const external_key_type& Name) { - DeclNameKey Key; - Key.Kind = Name.getNameKind(); - switch (Name.getNameKind()) { +unsigned DeclarationNameKey::getHash() const { + llvm::FoldingSetNodeID ID; + ID.AddInteger(Kind); + + switch (Kind) { case DeclarationName::Identifier: - Key.Data = (uint64_t)Name.getAsIdentifierInfo(); + case DeclarationName::CXXLiteralOperatorName: + ID.AddString(((IdentifierInfo*)Data)->getName()); break; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - Key.Data = (uint64_t)Name.getObjCSelector().getAsOpaquePtr(); + ID.AddInteger(serialization::ComputeHash(Selector(Data))); break; case DeclarationName::CXXOperatorName: - Key.Data = Name.getCXXOverloadedOperator(); - break; - case DeclarationName::CXXLiteralOperatorName: - Key.Data = (uint64_t)Name.getCXXLiteralIdentifier(); + ID.AddInteger((OverloadedOperatorKind)Data); break; case DeclarationName::CXXConstructorName: case DeclarationName::CXXDestructorName: case DeclarationName::CXXConversionFunctionName: case DeclarationName::CXXUsingDirective: - Key.Data = 0; break; } - return Key; + return ID.ComputeHash(); } std::pair<unsigned, unsigned> -ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char*& d) { +ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char *&d) { using namespace llvm::support; unsigned KeyLen = endian::readNext<uint16_t, little, unaligned>(d); unsigned DataLen = endian::readNext<uint16_t, little, unaligned>(d); return std::make_pair(KeyLen, DataLen); } -ASTDeclContextNameLookupTrait::internal_key_type -ASTDeclContextNameLookupTrait::ReadKey(const unsigned char* d, unsigned) { +ASTDeclContextNameLookupTrait::internal_key_type +ASTDeclContextNameLookupTrait::ReadKey(const unsigned char *d, unsigned) { using namespace llvm::support; - DeclNameKey Key; - Key.Kind = (DeclarationName::NameKind)*d++; - switch (Key.Kind) { + auto Kind = (DeclarationName::NameKind)*d++; + uint64_t Data; + switch (Kind) { case DeclarationName::Identifier: - Key.Data = (uint64_t)Reader.getLocalIdentifier( + Data = (uint64_t)Reader.getLocalIdentifier( F, endian::readNext<uint32_t, little, unaligned>(d)); break; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - Key.Data = + Data = (uint64_t)Reader.getLocalSelector( F, endian::readNext<uint32_t, little, unaligned>( d)).getAsOpaquePtr(); break; case DeclarationName::CXXOperatorName: - Key.Data = *d++; // OverloadedOperatorKind + Data = *d++; // OverloadedOperatorKind break; case DeclarationName::CXXLiteralOperatorName: - Key.Data = (uint64_t)Reader.getLocalIdentifier( + Data = (uint64_t)Reader.getLocalIdentifier( F, endian::readNext<uint32_t, little, unaligned>(d)); break; case DeclarationName::CXXConstructorName: case DeclarationName::CXXDestructorName: case DeclarationName::CXXConversionFunctionName: case DeclarationName::CXXUsingDirective: - Key.Data = 0; + Data = 0; break; } - return Key; + return DeclarationNameKey(Kind, Data); } ASTDeclContextNameLookupTrait::data_type @@ -6388,21 +6382,18 @@ namespace { ASTReader &Reader; const DeclContext *Context; DeclarationName Name; - ASTDeclContextNameLookupTrait::DeclNameKey NameKey; + DeclarationNameKey NameKey; unsigned NameHash; SmallVectorImpl<NamedDecl *> &Decls; llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet; public: - DeclContextNameLookupVisitor(ASTReader &Reader, - const DeclContext *Context, + DeclContextNameLookupVisitor(ASTReader &Reader, const DeclContext *Context, DeclarationName Name, SmallVectorImpl<NamedDecl *> &Decls, llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet) - : Reader(Reader), Context(Context), Name(Name), - NameKey(ASTDeclContextNameLookupTrait::GetInternalKey(Name)), - NameHash(ASTDeclContextNameLookupTrait::ComputeHash(NameKey)), - Decls(Decls), DeclSet(DeclSet) {} + : Reader(Reader), Context(Context), Name(Name), NameKey(Name), + NameHash(NameKey.getHash()), Decls(Decls), DeclSet(DeclSet) {} bool operator()(ModuleFile &M) { // Check whether we have any visible declaration information for @@ -6427,12 +6418,9 @@ namespace { continue; if (ND->getDeclName() != Name) { - // A name might be null because the decl's redeclarable part is - // currently read before reading its name. The lookup is triggered by - // building that decl (likely indirectly), and so it is later in the - // sense of "already existing" and can be ignored here. - // FIXME: This should not happen; deserializing declarations should - // not perform lookups since that can lead to deserialization cycles. + // Not all names map to a unique DeclarationNameKey. + assert(DeclarationNameKey(ND->getDeclName()) == NameKey && + "mismatched name for decl in decl context lookup table?"); continue; } Modified: cfe/trunk/lib/Serialization/ASTReaderInternals.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderInternals.h?rev=246124&r1=246123&r2=246124&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderInternals.h (original) +++ cfe/trunk/lib/Serialization/ASTReaderInternals.h Wed Aug 26 18:55:49 2015 @@ -48,35 +48,30 @@ public: typedef unsigned hash_value_type; typedef unsigned offset_type; - /// \brief Special internal key for declaration names. - /// The hash table creates keys for comparison; we do not create - /// a DeclarationName for the internal key to avoid deserializing types. - struct DeclNameKey { - DeclarationName::NameKind Kind; - uint64_t Data; - DeclNameKey() : Kind((DeclarationName::NameKind)0), Data(0) { } - }; - typedef DeclarationName external_key_type; - typedef DeclNameKey internal_key_type; + typedef DeclarationNameKey internal_key_type; explicit ASTDeclContextNameLookupTrait(ASTReader &Reader, ModuleFile &F) : Reader(Reader), F(F) { } static bool EqualKey(const internal_key_type& a, const internal_key_type& b) { - return a.Kind == b.Kind && a.Data == b.Data; + return a == b; } - static hash_value_type ComputeHash(const DeclNameKey &Key); - static internal_key_type GetInternalKey(const external_key_type& Name); + static hash_value_type ComputeHash(const internal_key_type &Key) { + return Key.getHash(); + } + static internal_key_type GetInternalKey(const external_key_type &Name) { + return Name; + } static std::pair<unsigned, unsigned> - ReadKeyDataLength(const unsigned char*& d); + ReadKeyDataLength(const unsigned char *&d); - internal_key_type ReadKey(const unsigned char* d, unsigned); + internal_key_type ReadKey(const unsigned char *d, unsigned); - data_type ReadData(internal_key_type, const unsigned char* d, + data_type ReadData(internal_key_type, const unsigned char *d, unsigned DataLen); }; Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=246124&r1=246123&r2=246124&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Aug 26 18:55:49 2015 @@ -3340,7 +3340,7 @@ class ASTDeclContextNameLookupTrait { ASTWriter &Writer; public: - typedef DeclarationName key_type; + typedef DeclarationNameKey key_type; typedef key_type key_type_ref; typedef DeclContext::lookup_result data_type; @@ -3351,42 +3351,17 @@ public: explicit ASTDeclContextNameLookupTrait(ASTWriter &Writer) : Writer(Writer) { } - hash_value_type ComputeHash(DeclarationName Name) { - llvm::FoldingSetNodeID ID; - ID.AddInteger(Name.getNameKind()); - - switch (Name.getNameKind()) { - case DeclarationName::Identifier: - ID.AddString(Name.getAsIdentifierInfo()->getName()); - break; - case DeclarationName::ObjCZeroArgSelector: - case DeclarationName::ObjCOneArgSelector: - case DeclarationName::ObjCMultiArgSelector: - ID.AddInteger(serialization::ComputeHash(Name.getObjCSelector())); - break; - case DeclarationName::CXXConstructorName: - case DeclarationName::CXXDestructorName: - case DeclarationName::CXXConversionFunctionName: - break; - case DeclarationName::CXXOperatorName: - ID.AddInteger(Name.getCXXOverloadedOperator()); - break; - case DeclarationName::CXXLiteralOperatorName: - ID.AddString(Name.getCXXLiteralIdentifier()->getName()); - case DeclarationName::CXXUsingDirective: - break; - } - - return ID.ComputeHash(); + hash_value_type ComputeHash(DeclarationNameKey Name) { + return Name.getHash(); } - std::pair<unsigned,unsigned> - EmitKeyDataLength(raw_ostream& Out, DeclarationName Name, - data_type_ref Lookup) { + std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &Out, + DeclarationNameKey Name, + data_type_ref Lookup) { using namespace llvm::support; endian::Writer<little> LE(Out); unsigned KeyLen = 1; - switch (Name.getNameKind()) { + switch (Name.getKind()) { case DeclarationName::Identifier: case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: @@ -3412,26 +3387,24 @@ public: return std::make_pair(KeyLen, DataLen); } - void EmitKey(raw_ostream& Out, DeclarationName Name, unsigned) { + void EmitKey(raw_ostream &Out, DeclarationNameKey Name, unsigned) { using namespace llvm::support; endian::Writer<little> LE(Out); - LE.write<uint8_t>(Name.getNameKind()); - switch (Name.getNameKind()) { + LE.write<uint8_t>(Name.getKind()); + switch (Name.getKind()) { case DeclarationName::Identifier: - LE.write<uint32_t>(Writer.getIdentifierRef(Name.getAsIdentifierInfo())); + case DeclarationName::CXXLiteralOperatorName: + LE.write<uint32_t>(Writer.getIdentifierRef(Name.getIdentifier())); return; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - LE.write<uint32_t>(Writer.getSelectorRef(Name.getObjCSelector())); + LE.write<uint32_t>(Writer.getSelectorRef(Name.getSelector())); return; case DeclarationName::CXXOperatorName: - assert(Name.getCXXOverloadedOperator() < NUM_OVERLOADED_OPERATORS && + assert(Name.getOperatorKind() < NUM_OVERLOADED_OPERATORS && "Invalid operator?"); - LE.write<uint8_t>(Name.getCXXOverloadedOperator()); - return; - case DeclarationName::CXXLiteralOperatorName: - LE.write<uint32_t>(Writer.getIdentifierRef(Name.getCXXLiteralIdentifier())); + LE.write<uint8_t>(Name.getOperatorKind()); return; case DeclarationName::CXXConstructorName: case DeclarationName::CXXDestructorName: @@ -3443,8 +3416,8 @@ public: llvm_unreachable("Invalid name kind?"); } - void EmitData(raw_ostream& Out, key_type_ref, - data_type Lookup, unsigned DataLen) { + void EmitData(raw_ostream &Out, key_type_ref, data_type Lookup, + unsigned DataLen) { using namespace llvm::support; endian::Writer<little> LE(Out); uint64_t Start = Out.tell(); (void)Start; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits