Skip to content

Commit a06c7e6

Browse files
zygoloidzygoloid
committed
[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. llvm-svn: 246124
1 parent 03439a8 commit a06c7e6

File tree

4 files changed

+113
-111
lines changed

4 files changed

+113
-111
lines changed

‎clang/include/clang/Serialization/ASTBitCodes.h

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#ifndef LLVM_CLANG_SERIALIZATION_ASTBITCODES_H
1818
#define LLVM_CLANG_SERIALIZATION_ASTBITCODES_H
1919

20+
#include "clang/AST/DeclarationName.h"
2021
#include "clang/AST/Type.h"
2122
#include "llvm/ADT/DenseMap.h"
2223
#include "llvm/Bitcode/BitCodes.h"
@@ -1480,6 +1481,51 @@ namespace clang {
14801481
}
14811482
};
14821483

1484+
/// \brief A key used when looking up entities by \ref DeclarationName.
1485+
///
1486+
/// Different \ref DeclarationNames are mapped to different keys, but the
1487+
/// same key can occasionally represent multiple names (for names that
1488+
/// contain types, in particular).
1489+
class DeclarationNameKey {
1490+
typedef unsigned NameKind;
1491+
1492+
NameKind Kind;
1493+
uint64_t Data;
1494+
1495+
public:
1496+
DeclarationNameKey() : Kind(), Data() {}
1497+
DeclarationNameKey(DeclarationName Name);
1498+
1499+
DeclarationNameKey(NameKind Kind, uint64_t Data)
1500+
: Kind(Kind), Data(Data) {}
1501+
1502+
NameKind getKind() const { return Kind; }
1503+
1504+
IdentifierInfo *getIdentifier() const {
1505+
assert(Kind == DeclarationName::Identifier ||
1506+
Kind == DeclarationName::CXXLiteralOperatorName);
1507+
return (IdentifierInfo *)Data;
1508+
}
1509+
Selector getSelector() const {
1510+
assert(Kind == DeclarationName::ObjCZeroArgSelector ||
1511+
Kind == DeclarationName::ObjCOneArgSelector ||
1512+
Kind == DeclarationName::ObjCMultiArgSelector);
1513+
return Selector(Data);
1514+
}
1515+
OverloadedOperatorKind getOperatorKind() const {
1516+
assert(Kind == DeclarationName::CXXOperatorName);
1517+
return (OverloadedOperatorKind)Data;
1518+
}
1519+
1520+
/// Compute a fingerprint of this key for use in on-disk hash table.
1521+
unsigned getHash() const;
1522+
1523+
friend bool operator==(const DeclarationNameKey &A,
1524+
const DeclarationNameKey &B) {
1525+
return A.Kind == B.Kind && A.Data == B.Data;
1526+
}
1527+
};
1528+
14831529
/// @}
14841530
}
14851531
} // end namespace clang

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 39 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -850,108 +850,102 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
850850
return II;
851851
}
852852

853-
unsigned
854-
ASTDeclContextNameLookupTrait::ComputeHash(const DeclNameKey &Key) {
855-
llvm::FoldingSetNodeID ID;
856-
ID.AddInteger(Key.Kind);
857-
858-
switch (Key.Kind) {
853+
DeclarationNameKey::DeclarationNameKey(DeclarationName Name)
854+
: Kind(Name.getNameKind()) {
855+
switch (Kind) {
859856
case DeclarationName::Identifier:
860-
case DeclarationName::CXXLiteralOperatorName:
861-
ID.AddString(((IdentifierInfo*)Key.Data)->getName());
857+
Data = (uint64_t)Name.getAsIdentifierInfo();
862858
break;
863859
case DeclarationName::ObjCZeroArgSelector:
864860
case DeclarationName::ObjCOneArgSelector:
865861
case DeclarationName::ObjCMultiArgSelector:
866-
ID.AddInteger(serialization::ComputeHash(Selector(Key.Data)));
862+
Data = (uint64_t)Name.getObjCSelector().getAsOpaquePtr();
867863
break;
868864
case DeclarationName::CXXOperatorName:
869-
ID.AddInteger((OverloadedOperatorKind)Key.Data);
865+
Data = Name.getCXXOverloadedOperator();
866+
break;
867+
case DeclarationName::CXXLiteralOperatorName:
868+
Data = (uint64_t)Name.getCXXLiteralIdentifier();
870869
break;
871870
case DeclarationName::CXXConstructorName:
872871
case DeclarationName::CXXDestructorName:
873872
case DeclarationName::CXXConversionFunctionName:
874873
case DeclarationName::CXXUsingDirective:
874+
Data = 0;
875875
break;
876876
}
877-
878-
return ID.ComputeHash();
879877
}
880878

881-
ASTDeclContextNameLookupTrait::internal_key_type
882-
ASTDeclContextNameLookupTrait::GetInternalKey(
883-
const external_key_type& Name) {
884-
DeclNameKey Key;
885-
Key.Kind = Name.getNameKind();
886-
switch (Name.getNameKind()) {
879+
unsigned DeclarationNameKey::getHash() const {
880+
llvm::FoldingSetNodeID ID;
881+
ID.AddInteger(Kind);
882+
883+
switch (Kind) {
887884
case DeclarationName::Identifier:
888-
Key.Data = (uint64_t)Name.getAsIdentifierInfo();
885+
case DeclarationName::CXXLiteralOperatorName:
886+
ID.AddString(((IdentifierInfo*)Data)->getName());
889887
break;
890888
case DeclarationName::ObjCZeroArgSelector:
891889
case DeclarationName::ObjCOneArgSelector:
892890
case DeclarationName::ObjCMultiArgSelector:
893-
Key.Data = (uint64_t)Name.getObjCSelector().getAsOpaquePtr();
891+
ID.AddInteger(serialization::ComputeHash(Selector(Data)));
894892
break;
895893
case DeclarationName::CXXOperatorName:
896-
Key.Data = Name.getCXXOverloadedOperator();
897-
break;
898-
case DeclarationName::CXXLiteralOperatorName:
899-
Key.Data = (uint64_t)Name.getCXXLiteralIdentifier();
894+
ID.AddInteger((OverloadedOperatorKind)Data);
900895
break;
901896
case DeclarationName::CXXConstructorName:
902897
case DeclarationName::CXXDestructorName:
903898
case DeclarationName::CXXConversionFunctionName:
904899
case DeclarationName::CXXUsingDirective:
905-
Key.Data = 0;
906900
break;
907901
}
908902

909-
return Key;
903+
return ID.ComputeHash();
910904
}
911905

912906
std::pair<unsigned, unsigned>
913-
ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char*& d) {
907+
ASTDeclContextNameLookupTrait::ReadKeyDataLength(const unsigned char *&d) {
914908
using namespace llvm::support;
915909
unsigned KeyLen = endian::readNext<uint16_t, little, unaligned>(d);
916910
unsigned DataLen = endian::readNext<uint16_t, little, unaligned>(d);
917911
return std::make_pair(KeyLen, DataLen);
918912
}
919913

920-
ASTDeclContextNameLookupTrait::internal_key_type
921-
ASTDeclContextNameLookupTrait::ReadKey(const unsigned char* d, unsigned) {
914+
ASTDeclContextNameLookupTrait::internal_key_type
915+
ASTDeclContextNameLookupTrait::ReadKey(const unsigned char *d, unsigned) {
922916
using namespace llvm::support;
923917

924-
DeclNameKey Key;
925-
Key.Kind = (DeclarationName::NameKind)*d++;
926-
switch (Key.Kind) {
918+
auto Kind = (DeclarationName::NameKind)*d++;
919+
uint64_t Data;
920+
switch (Kind) {
927921
case DeclarationName::Identifier:
928-
Key.Data = (uint64_t)Reader.getLocalIdentifier(
922+
Data = (uint64_t)Reader.getLocalIdentifier(
929923
F, endian::readNext<uint32_t, little, unaligned>(d));
930924
break;
931925
case DeclarationName::ObjCZeroArgSelector:
932926
case DeclarationName::ObjCOneArgSelector:
933927
case DeclarationName::ObjCMultiArgSelector:
934-
Key.Data =
928+
Data =
935929
(uint64_t)Reader.getLocalSelector(
936930
F, endian::readNext<uint32_t, little, unaligned>(
937931
d)).getAsOpaquePtr();
938932
break;
939933
case DeclarationName::CXXOperatorName:
940-
Key.Data = *d++; // OverloadedOperatorKind
934+
Data = *d++; // OverloadedOperatorKind
941935
break;
942936
case DeclarationName::CXXLiteralOperatorName:
943-
Key.Data = (uint64_t)Reader.getLocalIdentifier(
937+
Data = (uint64_t)Reader.getLocalIdentifier(
944938
F, endian::readNext<uint32_t, little, unaligned>(d));
945939
break;
946940
case DeclarationName::CXXConstructorName:
947941
case DeclarationName::CXXDestructorName:
948942
case DeclarationName::CXXConversionFunctionName:
949943
case DeclarationName::CXXUsingDirective:
950-
Key.Data = 0;
944+
Data = 0;
951945
break;
952946
}
953947

954-
return Key;
948+
return DeclarationNameKey(Kind, Data);
955949
}
956950

957951
ASTDeclContextNameLookupTrait::data_type
@@ -6388,21 +6382,18 @@ namespace {
63886382
ASTReader &Reader;
63896383
const DeclContext *Context;
63906384
DeclarationName Name;
6391-
ASTDeclContextNameLookupTrait::DeclNameKey NameKey;
6385+
DeclarationNameKey NameKey;
63926386
unsigned NameHash;
63936387
SmallVectorImpl<NamedDecl *> &Decls;
63946388
llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet;
63956389

63966390
public:
6397-
DeclContextNameLookupVisitor(ASTReader &Reader,
6398-
const DeclContext *Context,
6391+
DeclContextNameLookupVisitor(ASTReader &Reader, const DeclContext *Context,
63996392
DeclarationName Name,
64006393
SmallVectorImpl<NamedDecl *> &Decls,
64016394
llvm::SmallPtrSetImpl<NamedDecl *> &DeclSet)
6402-
: Reader(Reader), Context(Context), Name(Name),
6403-
NameKey(ASTDeclContextNameLookupTrait::GetInternalKey(Name)),
6404-
NameHash(ASTDeclContextNameLookupTrait::ComputeHash(NameKey)),
6405-
Decls(Decls), DeclSet(DeclSet) {}
6395+
: Reader(Reader), Context(Context), Name(Name), NameKey(Name),
6396+
NameHash(NameKey.getHash()), Decls(Decls), DeclSet(DeclSet) {}
64066397

64076398
bool operator()(ModuleFile &M) {
64086399
// Check whether we have any visible declaration information for
@@ -6427,12 +6418,9 @@ namespace {
64276418
continue;
64286419

64296420
if (ND->getDeclName() != Name) {
6430-
// A name might be null because the decl's redeclarable part is
6431-
// currently read before reading its name. The lookup is triggered by
6432-
// building that decl (likely indirectly), and so it is later in the
6433-
// sense of "already existing" and can be ignored here.
6434-
// FIXME: This should not happen; deserializing declarations should
6435-
// not perform lookups since that can lead to deserialization cycles.
6421+
// Not all names map to a unique DeclarationNameKey.
6422+
assert(DeclarationNameKey(ND->getDeclName()) == NameKey &&
6423+
"mismatched name for decl in decl context lookup table?");
64366424
continue;
64376425
}
64386426

‎clang/lib/Serialization/ASTReaderInternals.h

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,35 +48,30 @@ class ASTDeclContextNameLookupTrait {
4848
typedef unsigned hash_value_type;
4949
typedef unsigned offset_type;
5050

51-
/// \brief Special internal key for declaration names.
52-
/// The hash table creates keys for comparison; we do not create
53-
/// a DeclarationName for the internal key to avoid deserializing types.
54-
struct DeclNameKey {
55-
DeclarationName::NameKind Kind;
56-
uint64_t Data;
57-
DeclNameKey() : Kind((DeclarationName::NameKind)0), Data(0) { }
58-
};
59-
6051
typedef DeclarationName external_key_type;
61-
typedef DeclNameKey internal_key_type;
52+
typedef DeclarationNameKey internal_key_type;
6253

6354
explicit ASTDeclContextNameLookupTrait(ASTReader &Reader, ModuleFile &F)
6455
: Reader(Reader), F(F) { }
6556

6657
static bool EqualKey(const internal_key_type& a,
6758
const internal_key_type& b) {
68-
return a.Kind == b.Kind && a.Data == b.Data;
59+
return a == b;
6960
}
7061

71-
static hash_value_type ComputeHash(const DeclNameKey &Key);
72-
static internal_key_type GetInternalKey(const external_key_type& Name);
62+
static hash_value_type ComputeHash(const internal_key_type &Key) {
63+
return Key.getHash();
64+
}
65+
static internal_key_type GetInternalKey(const external_key_type &Name) {
66+
return Name;
67+
}
7368

7469
static std::pair<unsigned, unsigned>
75-
ReadKeyDataLength(const unsigned char*& d);
70+
ReadKeyDataLength(const unsigned char *&d);
7671

77-
internal_key_type ReadKey(const unsigned char* d, unsigned);
72+
internal_key_type ReadKey(const unsigned char *d, unsigned);
7873

79-
data_type ReadData(internal_key_type, const unsigned char* d,
74+
data_type ReadData(internal_key_type, const unsigned char *d,
8075
unsigned DataLen);
8176
};
8277

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 17 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,7 +3340,7 @@ class ASTDeclContextNameLookupTrait {
33403340
ASTWriter &Writer;
33413341

33423342
public:
3343-
typedef DeclarationName key_type;
3343+
typedef DeclarationNameKey key_type;
33443344
typedef key_type key_type_ref;
33453345

33463346
typedef DeclContext::lookup_result data_type;
@@ -3351,42 +3351,17 @@ class ASTDeclContextNameLookupTrait {
33513351

33523352
explicit ASTDeclContextNameLookupTrait(ASTWriter &Writer) : Writer(Writer) { }
33533353

3354-
hash_value_type ComputeHash(DeclarationName Name) {
3355-
llvm::FoldingSetNodeID ID;
3356-
ID.AddInteger(Name.getNameKind());
3357-
3358-
switch (Name.getNameKind()) {
3359-
case DeclarationName::Identifier:
3360-
ID.AddString(Name.getAsIdentifierInfo()->getName());
3361-
break;
3362-
case DeclarationName::ObjCZeroArgSelector:
3363-
case DeclarationName::ObjCOneArgSelector:
3364-
case DeclarationName::ObjCMultiArgSelector:
3365-
ID.AddInteger(serialization::ComputeHash(Name.getObjCSelector()));
3366-
break;
3367-
case DeclarationName::CXXConstructorName:
3368-
case DeclarationName::CXXDestructorName:
3369-
case DeclarationName::CXXConversionFunctionName:
3370-
break;
3371-
case DeclarationName::CXXOperatorName:
3372-
ID.AddInteger(Name.getCXXOverloadedOperator());
3373-
break;
3374-
case DeclarationName::CXXLiteralOperatorName:
3375-
ID.AddString(Name.getCXXLiteralIdentifier()->getName());
3376-
case DeclarationName::CXXUsingDirective:
3377-
break;
3378-
}
3379-
3380-
return ID.ComputeHash();
3354+
hash_value_type ComputeHash(DeclarationNameKey Name) {
3355+
return Name.getHash();
33813356
}
33823357

3383-
std::pair<unsigned,unsigned>
3384-
EmitKeyDataLength(raw_ostream& Out, DeclarationName Name,
3385-
data_type_ref Lookup) {
3358+
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &Out,
3359+
DeclarationNameKey Name,
3360+
data_type_ref Lookup) {
33863361
using namespace llvm::support;
33873362
endian::Writer<little> LE(Out);
33883363
unsigned KeyLen = 1;
3389-
switch (Name.getNameKind()) {
3364+
switch (Name.getKind()) {
33903365
case DeclarationName::Identifier:
33913366
case DeclarationName::ObjCZeroArgSelector:
33923367
case DeclarationName::ObjCOneArgSelector:
@@ -3412,26 +3387,24 @@ class ASTDeclContextNameLookupTrait {
34123387
return std::make_pair(KeyLen, DataLen);
34133388
}
34143389

3415-
void EmitKey(raw_ostream& Out, DeclarationName Name, unsigned) {
3390+
void EmitKey(raw_ostream &Out, DeclarationNameKey Name, unsigned) {
34163391
using namespace llvm::support;
34173392
endian::Writer<little> LE(Out);
3418-
LE.write<uint8_t>(Name.getNameKind());
3419-
switch (Name.getNameKind()) {
3393+
LE.write<uint8_t>(Name.getKind());
3394+
switch (Name.getKind()) {
34203395
case DeclarationName::Identifier:
3421-
LE.write<uint32_t>(Writer.getIdentifierRef(Name.getAsIdentifierInfo()));
3396+
case DeclarationName::CXXLiteralOperatorName:
3397+
LE.write<uint32_t>(Writer.getIdentifierRef(Name.getIdentifier()));
34223398
return;
34233399
case DeclarationName::ObjCZeroArgSelector:
34243400
case DeclarationName::ObjCOneArgSelector:
34253401
case DeclarationName::ObjCMultiArgSelector:
3426-
LE.write<uint32_t>(Writer.getSelectorRef(Name.getObjCSelector()));
3402+
LE.write<uint32_t>(Writer.getSelectorRef(Name.getSelector()));
34273403
return;
34283404
case DeclarationName::CXXOperatorName:
3429-
assert(Name.getCXXOverloadedOperator() < NUM_OVERLOADED_OPERATORS &&
3405+
assert(Name.getOperatorKind() < NUM_OVERLOADED_OPERATORS &&
34303406
"Invalid operator?");
3431-
LE.write<uint8_t>(Name.getCXXOverloadedOperator());
3432-
return;
3433-
case DeclarationName::CXXLiteralOperatorName:
3434-
LE.write<uint32_t>(Writer.getIdentifierRef(Name.getCXXLiteralIdentifier()));
3407+
LE.write<uint8_t>(Name.getOperatorKind());
34353408
return;
34363409
case DeclarationName::CXXConstructorName:
34373410
case DeclarationName::CXXDestructorName:
@@ -3443,8 +3416,8 @@ class ASTDeclContextNameLookupTrait {
34433416
llvm_unreachable("Invalid name kind?");
34443417
}
34453418

3446-
void EmitData(raw_ostream& Out, key_type_ref,
3447-
data_type Lookup, unsigned DataLen) {
3419+
void EmitData(raw_ostream &Out, key_type_ref, data_type Lookup,
3420+
unsigned DataLen) {
34483421
using namespace llvm::support;
34493422
endian::Writer<little> LE(Out);
34503423
uint64_t Start = Out.tell(); (void)Start;

0 commit comments

Comments
 (0)