From: Richard Smith Date: Wed, 26 Aug 2015 23:55:49 +0000 (+0000) Subject: [modules] The key to a DeclContext name lookup table is not actually a X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e36baf1b0ddd6744b6d4d1a31bf1793fb5416678;p=clang [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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@246124 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Serialization/ASTBitCodes.h b/include/clang/Serialization/ASTBitCodes.h index 418eb076c4..eb8b99bf34 100644 --- a/include/clang/Serialization/ASTBitCodes.h +++ b/include/clang/Serialization/ASTBitCodes.h @@ -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 diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index f9522aeaa3..af1130df3f 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -850,108 +850,102 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k, 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 -ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char*& d) { +ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char *&d) { using namespace llvm::support; unsigned KeyLen = endian::readNext(d); unsigned DataLen = endian::readNext(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(d)); break; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - Key.Data = + Data = (uint64_t)Reader.getLocalSelector( F, endian::readNext( 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(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 &Decls; llvm::SmallPtrSetImpl &DeclSet; public: - DeclContextNameLookupVisitor(ASTReader &Reader, - const DeclContext *Context, + DeclContextNameLookupVisitor(ASTReader &Reader, const DeclContext *Context, DeclarationName Name, SmallVectorImpl &Decls, llvm::SmallPtrSetImpl &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; } diff --git a/lib/Serialization/ASTReaderInternals.h b/lib/Serialization/ASTReaderInternals.h index 47640f6c47..9e097000f2 100644 --- a/lib/Serialization/ASTReaderInternals.h +++ b/lib/Serialization/ASTReaderInternals.h @@ -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 - 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); }; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 9563b385e9..b5340dc41d 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -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 - EmitKeyDataLength(raw_ostream& Out, DeclarationName Name, - data_type_ref Lookup) { + std::pair EmitKeyDataLength(raw_ostream &Out, + DeclarationNameKey Name, + data_type_ref Lookup) { using namespace llvm::support; endian::Writer 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 LE(Out); - LE.write(Name.getNameKind()); - switch (Name.getNameKind()) { + LE.write(Name.getKind()); + switch (Name.getKind()) { case DeclarationName::Identifier: - LE.write(Writer.getIdentifierRef(Name.getAsIdentifierInfo())); + case DeclarationName::CXXLiteralOperatorName: + LE.write(Writer.getIdentifierRef(Name.getIdentifier())); return; case DeclarationName::ObjCZeroArgSelector: case DeclarationName::ObjCOneArgSelector: case DeclarationName::ObjCMultiArgSelector: - LE.write(Writer.getSelectorRef(Name.getObjCSelector())); + LE.write(Writer.getSelectorRef(Name.getSelector())); return; case DeclarationName::CXXOperatorName: - assert(Name.getCXXOverloadedOperator() < NUM_OVERLOADED_OPERATORS && + assert(Name.getOperatorKind() < NUM_OVERLOADED_OPERATORS && "Invalid operator?"); - LE.write(Name.getCXXOverloadedOperator()); - return; - case DeclarationName::CXXLiteralOperatorName: - LE.write(Writer.getIdentifierRef(Name.getCXXLiteralIdentifier())); + LE.write(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 LE(Out); uint64_t Start = Out.tell(); (void)Start;