replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[modules] In C++, stop serializing and deserializing a list of declarations in 

the identifier table. This is redundant, since the TU-scope lookups are also
serialized as part of the TU DeclContext, and wasteful in a number of ways. We
still emit the decls for PCH / preamble builds, since for those we want
identical results, not merely semantically equivalent ones.

llvm-svn: 242855
This commit is contained in:
Richard Smith 2015-07-21 23:54:07 +00:00
parent 159e5ed9dc
commit a534a31c5e
9 changed files with 94 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
clang/include/clang/AST/DeclObjC.h
View File

@ -1202,13 +1202,8 @@ public:
// might bring in a definition.
// Note: a null value indicates that we don't have a definition and that
// modules are enabled.
if (!Data.getOpaqueValue()) {
if (IdentifierInfo *II = getIdentifier()) {
if (II->isOutOfDate()) {
updateOutOfDate(*II);
}
}
}
if (!Data.getOpaqueValue())
getMostRecentDecl();
return Data.getPointer();
}
@ -1851,13 +1846,8 @@ public:
// might bring in a definition.
// Note: a null value indicates that we don't have a definition and that
// modules are enabled.
if (!Data.getOpaqueValue()) {
if (IdentifierInfo *II = getIdentifier()) {
if (II->isOutOfDate()) {
updateOutOfDate(*II);
}
}
}
if (!Data.getOpaqueValue())
getMostRecentDecl();
return Data.getPointer();
}

23
clang/include/clang/Sema/Lookup.h
View File

@ -140,6 +140,7 @@ public:
HideTags(true),
Diagnose(Redecl == Sema::NotForRedeclaration),
AllowHidden(Redecl == Sema::ForRedeclaration),
AllowHiddenInternal(AllowHidden),
Shadowed(false)
{
configure();
@ -162,6 +163,7 @@ public:
HideTags(true),
Diagnose(Redecl == Sema::NotForRedeclaration),
AllowHidden(Redecl == Sema::ForRedeclaration),
AllowHiddenInternal(AllowHidden),
Shadowed(false)
{
configure();
@ -182,6 +184,7 @@ public:
HideTags(Other.HideTags),
Diagnose(false),
AllowHidden(Other.AllowHidden),
AllowHiddenInternal(Other.AllowHiddenInternal),
Shadowed(false)
{}
@ -223,13 +226,20 @@ public:
/// \brief Specify whether hidden declarations are visible, e.g.,
/// for recovery reasons.
void setAllowHidden(bool AH) {
AllowHidden = AH;
AllowHiddenInternal = AllowHidden = AH;
}
/// \brief Specify whether hidden internal declarations are visible.
void setAllowHiddenInternal(bool AHI) {
AllowHiddenInternal = AHI;
}
/// \brief Determine whether this lookup is permitted to see hidden
/// declarations, such as those in modules that have not yet been imported.
bool isHiddenDeclarationVisible() const {
return AllowHidden || LookupKind == Sema::LookupTagName;
bool isHiddenDeclarationVisible(NamedDecl *ND) const {
return (AllowHidden &&
(AllowHiddenInternal || ND->isExternallyVisible())) ||
LookupKind == Sema::LookupTagName;
}
/// Sets whether tag declarations should be hidden by non-tag
@ -302,7 +312,7 @@ public:
if (!D->isInIdentifierNamespace(IDNS))
return nullptr;
if (isHiddenDeclarationVisible() || isVisible(getSema(), D))
if (isHiddenDeclarationVisible(D) || isVisible(getSema(), D))
return D;
return getAcceptableDeclSlow(D);
@ -511,7 +521,7 @@ public:
/// \brief Change this lookup's redeclaration kind.
void setRedeclarationKind(Sema::RedeclarationKind RK) {
Redecl = RK;
AllowHidden = (RK == Sema::ForRedeclaration);
AllowHiddenInternal = AllowHidden = (RK == Sema::ForRedeclaration);
configure();
}
@ -678,6 +688,9 @@ private:
/// \brief True if we should allow hidden declarations to be 'visible'.
bool AllowHidden;
/// \brief True if we should allow hidden internal declarations to be visible.
bool AllowHiddenInternal;
/// \brief True if the found declarations were shadowed by some other
/// declaration that we skipped. This only happens when \c LookupKind
/// is \c LookupRedeclarationWithLinkage.

45
clang/lib/Sema/SemaDecl.cpp
View File

@ -1796,37 +1796,6 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
return New;
}
/// \brief Filter out any previous declarations that the given declaration
/// should not consider because they are not permitted to conflict, e.g.,
/// because they come from hidden sub-modules and do not refer to the same
/// entity.
static void filterNonConflictingPreviousDecls(Sema &S,
NamedDecl *decl,
LookupResult &previous){
// This is only interesting when modules are enabled.
if ((!S.getLangOpts().Modules && !S.getLangOpts().ModulesLocalVisibility) ||
!S.getLangOpts().ModulesHideInternalLinkage)
return;
// Empty sets are uninteresting.
if (previous.empty())
return;
LookupResult::Filter filter = previous.makeFilter();
while (filter.hasNext()) {
NamedDecl *old = filter.next();
// Non-hidden declarations are never ignored.
if (S.isVisible(old))
continue;
if (!old->isExternallyVisible())
filter.erase();
}
filter.done();
}
/// Typedef declarations don't have linkage, but they still denote the same
/// entity if their types are the same.
/// FIXME: This is notionally doing the same thing as ASTReaderDecl's
@ -4801,6 +4770,13 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
ForRedeclaration);
// If we're hiding internal-linkage symbols in modules from redeclaration
// lookup, let name lookup know.
if ((getLangOpts().Modules || getLangOpts().ModulesLocalVisibility) &&
getLangOpts().ModulesHideInternalLinkage &&
D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef)
Previous.setAllowHiddenInternal(false);
// See if this is a redefinition of a variable in the same scope.
if (!D.getCXXScopeSpec().isSet()) {
bool IsLinkageLookup = false;
@ -6500,9 +6476,6 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous) {
checkForConflictWithNonVisibleExternC(*this, NewVD, Previous))
Previous.setShadowed();
// Filter out any non-conflicting previous declarations.
filterNonConflictingPreviousDecls(*this, NewVD, Previous);
if (!Previous.empty()) {
MergeVarDecl(NewVD, Previous);
return true;
@ -8058,9 +8031,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
bool MergeTypeWithPrevious = !getLangOpts().CPlusPlus &&
!Previous.isShadowed();
// Filter out any non-conflicting previous declarations.
filterNonConflictingPreviousDecls(*this, NewFD, Previous);
bool Redeclaration = false;
NamedDecl *OldDecl = nullptr;
@ -8114,7 +8084,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
// Check for a previous extern "C" declaration with this name.
if (!Redeclaration &&
checkForConflictWithNonVisibleExternC(*this, NewFD, Previous)) {
filterNonConflictingPreviousDecls(*this, NewFD, Previous);
if (!Previous.empty()) {
// This is an extern "C" declaration with the same name as a previous
// declaration, and thus redeclares that entity...

26
clang/lib/Sema/SemaLookup.cpp
View File

@ -378,7 +378,7 @@ void LookupResult::resolveKind() {
// Don't do any extra resolution if we've already resolved as ambiguous.
if (ResultKind == Ambiguous) return;
llvm::SmallPtrSet<NamedDecl*, 16> Unique;
llvm::SmallDenseMap<NamedDecl*, unsigned, 16> Unique;
llvm::SmallPtrSet<QualType, 16> UniqueTypes;
bool Ambiguous = false;
@ -414,14 +414,26 @@ void LookupResult::resolveKind() {
}
}
if (!Unique.insert(D).second) {
auto UniqueResult = Unique.insert(std::make_pair(D, I));
if (!UniqueResult.second) {
// If it's not unique, pull something off the back (and
// continue at this index).
// FIXME: This is wrong. We need to take the more recent declaration in
// order to get the right type, default arguments, etc. We also need to
// prefer visible declarations to hidden ones (for redeclaration lookup
// in modules builds).
Decls[I] = Decls[--N];
auto ExistingI = UniqueResult.first->second;
auto *Existing = Decls[ExistingI]->getUnderlyingDecl();
for (Decl *Prev = Decls[I]->getUnderlyingDecl()->getPreviousDecl(); /**/;
Prev = Prev->getPreviousDecl()) {
if (Prev == Existing) {
// Existing result is older. Replace it with the new one.
Decls[ExistingI] = Decls[I];
Decls[I] = Decls[--N];
break;
}
if (!Prev) {
// New decl is older. Keep the existing one.
Decls[I] = Decls[--N];
break;
}
}
continue;
}

19
clang/lib/Serialization/ASTReader.cpp
View File

@ -735,12 +735,14 @@ ASTIdentifierLookupTraitBase::ReadKey(const unsigned char* d, unsigned n) {
}
/// \brief Whether the given identifier is "interesting".
static bool isInterestingIdentifier(IdentifierInfo &II, bool IsModule) {
static bool isInterestingIdentifier(ASTReader &Reader, IdentifierInfo &II,
bool IsModule) {
return II.hadMacroDefinition() ||
II.isPoisoned() ||
(IsModule ? II.hasRevertedBuiltin() : II.getObjCOrBuiltinID()) ||
II.hasRevertedTokenIDToIdentifier() ||
II.getFETokenInfo<void>();
(!(IsModule && Reader.getContext().getLangOpts().CPlusPlus) &&
II.getFETokenInfo<void>());
}
static bool readBit(unsigned &Bits) {
@ -767,7 +769,7 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
}
if (!II->isFromAST()) {
II->setIsFromAST();
if (isInterestingIdentifier(*II, F.isModule()))
if (isInterestingIdentifier(Reader, *II, F.isModule()))
II->setChangedSinceDeserialization();
}
Reader.markIdentifierUpToDate(II);
@ -5883,10 +5885,13 @@ void ASTReader::CompleteRedeclChain(const Decl *D) {
if (isa<TranslationUnitDecl>(DC) || isa<NamespaceDecl>(DC) ||
isa<CXXRecordDecl>(DC) || isa<EnumDecl>(DC)) {
if (DeclarationName Name = cast<NamedDecl>(D)->getDeclName()) {
auto *II = Name.getAsIdentifierInfo();
if (isa<TranslationUnitDecl>(DC) && II) {
if (!getContext().getLangOpts().CPlusPlus &&
isa<TranslationUnitDecl>(DC)) {
// Outside of C++, we don't have a lookup table for the TU, so update
// the identifier instead. In C++, either way should work fine.
// the identifier instead. (For C++ modules, we don't store decls
// in the serialized identifier table, so we do the lookup in the TU.)
auto *II = Name.getAsIdentifierInfo();
assert(II && "non-identifier name in C?");
if (II->isOutOfDate())
updateOutOfDateIdentifier(*II);
} else
@ -8443,7 +8448,7 @@ void ASTReader::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
// Remove any fake results before adding any real ones.
auto It = PendingFakeLookupResults.find(II);
if (It != PendingFakeLookupResults.end()) {
for (auto *ND : PendingFakeLookupResults[II])
for (auto *ND : It->second)
SemaObj->IdResolver.RemoveDecl(ND);
// FIXME: this works around module+PCH performance issue.
// Rather than erase the result from the map, which is O(n), just clear

43
clang/lib/Serialization/ASTWriter.cpp
View File

@ -3103,6 +3103,7 @@ class ASTIdentifierTableTrait {
Preprocessor &PP;
IdentifierResolver &IdResolver;
bool IsModule;
bool NeedDecls;
/// \brief Determines whether this is an "interesting" identifier that needs a
/// full IdentifierInfo structure written into the hash table. Notably, this
@ -3113,7 +3114,7 @@ class ASTIdentifierTableTrait {
II->isPoisoned() ||
(IsModule ? II->hasRevertedBuiltin() : II->getObjCOrBuiltinID()) ||
II->hasRevertedTokenIDToIdentifier() ||
II->getFETokenInfo<void>())
(NeedDecls && II->getFETokenInfo<void>()))
return true;
return false;
@ -3131,7 +3132,8 @@ public:
ASTIdentifierTableTrait(ASTWriter &Writer, Preprocessor &PP,
IdentifierResolver &IdResolver, bool IsModule)
: Writer(Writer), PP(PP), IdResolver(IdResolver), IsModule(IsModule) {}
: Writer(Writer), PP(PP), IdResolver(IdResolver), IsModule(IsModule),
NeedDecls(!IsModule || !Writer.getLangOpts().CPlusPlus) {}
static hash_value_type ComputeHash(const IdentifierInfo* II) {
return llvm::HashString(II->getName());
@ -3152,10 +3154,12 @@ public:
if (MacroOffset)
DataLen += 4; // MacroDirectives offset.
for (IdentifierResolver::iterator D = IdResolver.begin(II),
DEnd = IdResolver.end();
D != DEnd; ++D)
DataLen += 4;
if (NeedDecls) {
for (IdentifierResolver::iterator D = IdResolver.begin(II),
DEnd = IdResolver.end();
D != DEnd; ++D)
DataLen += 4;
}
}
using namespace llvm::support;
endian::Writer<little> LE(Out);
@ -3205,18 +3209,21 @@ public:
if (HadMacroDefinition)
LE.write<uint32_t>(MacroOffset);
// Emit the declaration IDs in reverse order, because the
// IdentifierResolver provides the declarations as they would be
// visible (e.g., the function "stat" would come before the struct
// "stat"), but the ASTReader adds declarations to the end of the list
// (so we need to see the struct "stat" before the function "stat").
// Only emit declarations that aren't from a chained PCH, though.
SmallVector<NamedDecl *, 16> Decls(IdResolver.begin(II), IdResolver.end());
for (SmallVectorImpl<NamedDecl *>::reverse_iterator D = Decls.rbegin(),
DEnd = Decls.rend();
D != DEnd; ++D)
LE.write<uint32_t>(
Writer.getDeclID(getDeclForLocalLookup(PP.getLangOpts(), *D)));
if (NeedDecls) {
// Emit the declaration IDs in reverse order, because the
// IdentifierResolver provides the declarations as they would be
// visible (e.g., the function "stat" would come before the struct
// "stat"), but the ASTReader adds declarations to the end of the list
// (so we need to see the struct "stat" before the function "stat").
// Only emit declarations that aren't from a chained PCH, though.
SmallVector<NamedDecl *, 16> Decls(IdResolver.begin(II),
IdResolver.end());
for (SmallVectorImpl<NamedDecl *>::reverse_iterator D = Decls.rbegin(),
DEnd = Decls.rend();
D != DEnd; ++D)
LE.write<uint32_t>(
Writer.getDeclID(getDeclForLocalLookup(PP.getLangOpts(), *D)));
}
}
};
} // end anonymous namespace

5
clang/test/CXX/class.access/class.friend/p1.cpp
View File

@ -8,11 +8,12 @@
// friends members of the befriending class.
struct S { static void f(); }; // expected-note 2 {{'S' declared here}}
S* g() { return 0; } // expected-note 2 {{'g' declared here}}
S* g() { return 0; }
struct X {
friend struct S;
friend S* g();
friend S* g(); // expected-note 2 {{'g' declared here}}
// FIXME: The above two notes would be better attached to line 11.
};
void test1() {

11
clang/test/CXX/class.access/class.friend/p11.cpp
View File

@ -24,13 +24,12 @@ namespace test2 {
void foo() { // expected-note {{'::test2::foo' declared here}}
struct S1 {
friend void foo(); // expected-error {{no matching function 'foo' found in local scope; did you mean '::test2::foo'?}}
// expected-note@-1{{'::test2::foo' declared here}}
// TODO: the above note should go on line 24
};
void foo(); // expected-note {{local declaration nearly matches}}
struct S2 {
friend void foo();
friend void foo(); // expected-note{{'::test2::foo' declared here}}
// TODO: the above note should go on line 24
};
{
@ -48,8 +47,8 @@ namespace test2 {
struct S4 {
friend void bar(); // expected-error {{no matching function 'bar' found in local scope; did you mean '::test2::bar'?}}
// expected-note@-1 2 {{'::test2::bar' declared here}}
// TODO: the above two notes should go on line 22
// expected-note@-1 {{'::test2::bar' declared here}}
// TODO: the above note should go on line 22
};
{ void bar(); }
@ -82,6 +81,8 @@ namespace test2 {
struct S9 {
struct Inner {
friend void baz(); // expected-error {{no matching function 'baz' found in local scope; did you mean 'bar'?}}
// expected-note@-1 {{'::test2::bar' declared here}}
// TODO: the above note should go on line 22
};
};

4
clang/test/Modules/linkage-merge.cpp
View File

@ -7,9 +7,5 @@ static int f(int);
int f(int);
static void g(int);
// FIXME: Whether we notice the problem here depends on the order in which we
// happen to find lookup results for 'g'; LookupResult::resolveKind needs to
// be taught to prefer a visible result over a non-visible one.
//
// expected-error@9 {{functions that differ only in their return type cannot be overloaded}}
// expected-note@Inputs/linkage-merge-foo.h:2 {{previous declaration is here}}