r232793 - [modules] Remove some redundant work when building a lookup table for a DeclContext.
Richard Smith
richard-llvm at metafoo.co.uk
Thu Mar 19 19:17:22 PDT 2015
Author: rsmith
Date: Thu Mar 19 21:17:21 2015
New Revision: 232793
URL: http://llvm.org/viewvc/llvm-project?rev=232793&view=rev
Log:
[modules] Remove some redundant work when building a lookup table for a DeclContext.
When we need to build the lookup table for a DeclContext, we used to pull in
all lexical declarations for the context; instead, just build a lookup table
for the local lexical declarations. We previously didn't guarantee that the
imported declarations would be in the returned map, but in some cases we'd
happen to put them all in there regardless. Now we're even lazier about this.
This unnecessary work was papering over some other bugs:
- LookupVisibleDecls would use the DC for name lookups in the TU in C, and
this was not guaranteed to find all imported names (generally, the DC for
the TU in C is not a reliable place to perform lookups). We now use an
identifier-based lookup mechanism for this.
- We didn't actually load in the list of eagerly-deserialized declarations
when importing a module (so external definitions in a module wouldn't be
emitted by users of those modules unless they happened to be deserialized
by the user of the module).
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Sema/IdentifierResolver.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/cxx-templates.cpp
cfe/trunk/test/Modules/odr.cpp
cfe/trunk/test/Modules/redecl-add-after-load.cpp
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Thu Mar 19 21:17:21 2015
@@ -1722,8 +1722,6 @@ private:
friend class DependentDiagnostic;
StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;
- template<decl_iterator (DeclContext::*Begin)() const,
- decl_iterator (DeclContext::*End)() const>
void buildLookupImpl(DeclContext *DCtx, bool Internal);
void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
bool Rediscoverable);
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Mar 19 21:17:21 2015
@@ -7946,7 +7946,7 @@ static GVALinkage basicGVALinkageForVari
while (LexicalContext && !isa<FunctionDecl>(LexicalContext))
LexicalContext = LexicalContext->getLexicalParent();
- // Let the static local variable inherit it's linkage from the nearest
+ // Let the static local variable inherit its linkage from the nearest
// enclosing function.
if (LexicalContext)
StaticLocalLinkage =
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Thu Mar 19 21:17:21 2015
@@ -1263,15 +1263,13 @@ static bool shouldBeHidden(NamedDecl *D)
StoredDeclsMap *DeclContext::buildLookup() {
assert(this == getPrimaryContext() && "buildLookup called on non-primary DC");
- // FIXME: Should we keep going if hasExternalVisibleStorage?
if (!LookupPtr.getInt())
return LookupPtr.getPointer();
SmallVector<DeclContext *, 2> Contexts;
collectAllContexts(Contexts);
for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
- buildLookupImpl<&DeclContext::decls_begin,
- &DeclContext::decls_end>(Contexts[I], false);
+ buildLookupImpl(Contexts[I], hasExternalVisibleStorage());
// We no longer have any lazy decls.
LookupPtr.setInt(false);
@@ -1282,13 +1280,8 @@ StoredDeclsMap *DeclContext::buildLookup
/// declarations contained within DCtx, which will either be this
/// DeclContext, a DeclContext linked to it, or a transparent context
/// nested within it.
-template<DeclContext::decl_iterator (DeclContext::*Begin)() const,
- DeclContext::decl_iterator (DeclContext::*End)() const>
void DeclContext::buildLookupImpl(DeclContext *DCtx, bool Internal) {
- for (decl_iterator I = (DCtx->*Begin)(), E = (DCtx->*End)();
- I != E; ++I) {
- Decl *D = *I;
-
+ for (Decl *D : DCtx->noload_decls()) {
// Insert this declaration into the lookup structure, but only if
// it's semantically within its decl context. Any other decls which
// should be found in this context are added eagerly.
@@ -1309,7 +1302,7 @@ void DeclContext::buildLookupImpl(DeclCo
// context (recursively).
if (DeclContext *InnerCtx = dyn_cast<DeclContext>(D))
if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace())
- buildLookupImpl<Begin, End>(InnerCtx, Internal);
+ buildLookupImpl(InnerCtx, Internal);
}
}
@@ -1388,26 +1381,8 @@ DeclContext::noload_lookup(DeclarationNa
if (PrimaryContext != this)
return PrimaryContext->noload_lookup(Name);
- StoredDeclsMap *Map = LookupPtr.getPointer();
- if (LookupPtr.getInt()) {
- // Carefully build the lookup map, without deserializing anything.
- SmallVector<DeclContext *, 2> Contexts;
- collectAllContexts(Contexts);
- for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
- buildLookupImpl<&DeclContext::noload_decls_begin,
- &DeclContext::noload_decls_end>(Contexts[I], true);
-
- // We no longer have any lazy decls.
- LookupPtr.setInt(false);
-
- // There may now be names for which we have local decls but are
- // missing the external decls. FIXME: Just set the hasExternalDecls
- // flag on those names that have external decls.
- NeedToReconcileExternalVisibleStorage = true;
-
- Map = LookupPtr.getPointer();
- }
-
+ // Note that buildLookups does not trigger any deserialization.
+ StoredDeclsMap *Map = buildLookup();
if (!Map)
return lookup_result();
Modified: cfe/trunk/lib/Sema/IdentifierResolver.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/IdentifierResolver.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/IdentifierResolver.cpp (original)
+++ cfe/trunk/lib/Sema/IdentifierResolver.cpp Thu Mar 19 21:17:21 2015
@@ -98,7 +98,7 @@ bool IdentifierResolver::isDeclInScope(D
bool AllowInlineNamespace) const {
Ctx = Ctx->getRedeclContext();
- if (Ctx->isFunctionOrMethod() || S->isFunctionPrototypeScope()) {
+ if (Ctx->isFunctionOrMethod() || (S && S->isFunctionPrototypeScope())) {
// Ignore the scopes associated within transparent declaration contexts.
while (S->getEntity() && S->getEntity()->isTransparentContext())
S = S->getParent();
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Mar 19 21:17:21 2015
@@ -3021,17 +3021,45 @@ static void LookupVisibleDecls(DeclConte
if (Visited.visitedContext(Ctx->getPrimaryContext()))
return;
+ // Outside C++, lookup results for the TU live on identifiers.
+ if (isa<TranslationUnitDecl>(Ctx) &&
+ !Result.getSema().getLangOpts().CPlusPlus) {
+ auto &S = Result.getSema();
+ auto &Idents = S.Context.Idents;
+
+ // Ensure all external identifiers are in the identifier table.
+ if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
+ std::unique_ptr<IdentifierIterator> Iter(External->getIdentifiers());
+ for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
+ Idents.get(Name);
+ }
+
+ // Walk all lookup results in the TU for each identifier.
+ for (const auto &Ident : Idents) {
+ for (auto I = S.IdResolver.begin(Ident.getValue()),
+ E = S.IdResolver.end();
+ I != E; ++I) {
+ if (S.IdResolver.isDeclInScope(*I, Ctx)) {
+ if (NamedDecl *ND = Result.getAcceptableDecl(*I)) {
+ Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+ Visited.add(ND);
+ }
+ }
+ }
+ }
+
+ return;
+ }
+
if (CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(Ctx))
Result.getSema().ForceDeclarationOfImplicitMembers(Class);
// Enumerate all of the results in this context.
for (const auto &R : Ctx->lookups()) {
- for (auto *I : R) {
- if (NamedDecl *ND = dyn_cast<NamedDecl>(I)) {
- if ((ND = Result.getAcceptableDecl(ND))) {
- Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
- Visited.add(ND);
- }
+ for (auto *D : R) {
+ if (auto *ND = Result.getAcceptableDecl(D)) {
+ Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+ Visited.add(ND);
}
}
}
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Mar 19 21:17:21 2015
@@ -2835,6 +2835,8 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
}
case EAGERLY_DESERIALIZED_DECLS:
+ // FIXME: Skip reading this record if our ASTConsumer doesn't care
+ // about "interesting" decls (for instance, if we're building a module).
for (unsigned I = 0, N = Record.size(); I != N; ++I)
EagerlyDeserializedDecls.push_back(getGlobalDeclID(F, Record[I]));
break;
@@ -6832,6 +6834,12 @@ void ASTReader::PassInterestingDeclsToCo
SaveAndRestore<bool> GuardPassingDeclsToConsumer(PassingDeclsToConsumer,
true);
+ // Ensure that we've loaded all potentially-interesting declarations
+ // that need to be eagerly loaded.
+ for (auto ID : EagerlyDeserializedDecls)
+ GetDecl(ID);
+ EagerlyDeserializedDecls.clear();
+
while (!InterestingDecls.empty()) {
Decl *D = InterestingDecls.front();
InterestingDecls.pop_front();
@@ -6850,17 +6858,8 @@ void ASTReader::PassInterestingDeclToCon
void ASTReader::StartTranslationUnit(ASTConsumer *Consumer) {
this->Consumer = Consumer;
- if (!Consumer)
- return;
-
- for (unsigned I = 0, N = EagerlyDeserializedDecls.size(); I != N; ++I) {
- // Force deserialization of this decl, which will cause it to be queued for
- // passing to the consumer.
- GetDecl(EagerlyDeserializedDecls[I]);
- }
- EagerlyDeserializedDecls.clear();
-
- PassInterestingDeclsToConsumer();
+ if (Consumer)
+ PassInterestingDeclsToConsumer();
if (DeserializationListener)
DeserializationListener->ReaderInitialized(this);
Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Thu Mar 19 21:17:21 2015
@@ -1,7 +1,7 @@
// RUN: rm -rf %t
// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump-lookups | FileCheck %s --check-prefix=CHECK-GLOBAL
// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump-lookups -ast-dump-filter N | FileCheck %s --check-prefix=CHECK-NAMESPACE-N
-// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump | FileCheck %s --check-prefix=CHECK-DUMP
+// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-filter SomeTemplate | FileCheck %s --check-prefix=CHECK-DUMP
// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11
// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DEARLY_IMPORT
@@ -125,9 +125,10 @@ void g() {
static_assert(Outer<int>::Inner<int>::f() == 1, "");
static_assert(Outer<int>::Inner<int>::g() == 2, "");
-#ifndef EARLY_IMPORT
-// FIXME: The textual inclusion above shouldn't cause this to fail!
-static_assert(MergeTemplateDefinitions<int>::f() == 1, "");
+// FIXME: We're too lazy in merging class definitions to find the definition
+// of this function.
+static_assert(MergeTemplateDefinitions<int>::f() == 1, ""); // expected-error {{constant expression}} expected-note {{undefined}}
+// expected-note at cxx-templates-c.h:10 {{here}}
static_assert(MergeTemplateDefinitions<int>::g() == 2, "");
RedeclaredAsFriend<int> raf1;
@@ -140,7 +141,6 @@ MergeSpecializations<int[]>::partially_s
MergeSpecializations<char>::explicitly_specialized_in_a spec_in_a_2;
MergeSpecializations<double>::explicitly_specialized_in_b spec_in_b_2;
MergeSpecializations<bool>::explicitly_specialized_in_c spec_in_c_2;
-#endif
MergeAnonUnionMember<> maum_main;
typedef DontWalkPreviousDeclAfterMerging<int> dwpdam_typedef_2;
Modified: cfe/trunk/test/Modules/odr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/test/Modules/odr.cpp (original)
+++ cfe/trunk/test/Modules/odr.cpp Thu Mar 19 21:17:21 2015
@@ -15,9 +15,9 @@ bool b = F<int>{0} == F<int>{1};
int x = f() + g();
// expected-note at a.h:5 {{definition has no member 'e2'}}
-// expected-note at a.h:3 {{declaration of 'f' does not match}}
-// expected-note at a.h:1 {{definition has no member 'm'}}
+// expected-note at b.h:3 {{declaration of 'f' does not match}}
+// expected-note at b.h:1 {{definition has no member 'n'}}
// expected-error at b.h:5 {{'E::e2' from module 'b' is not present in definition of 'E' in module 'a'}}
-// expected-error at b.h:3 {{'Y::f' from module 'b' is not present in definition of 'Y' in module 'a'}}
-// expected-error at b.h:2 {{'Y::m' from module 'b' is not present in definition of 'Y' in module 'a'}}
+// expected-error at a.h:3 {{'Y::f' from module 'a' is not present in definition of 'Y' in module 'b'}}
+// expected-error at a.h:2 {{'Y::n' from module 'a' is not present in definition of 'Y' in module 'b'}}
Modified: cfe/trunk/test/Modules/redecl-add-after-load.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/redecl-add-after-load.cpp?rev=232793&r1=232792&r2=232793&view=diff
==============================================================================
--- cfe/trunk/test/Modules/redecl-add-after-load.cpp (original)
+++ cfe/trunk/test/Modules/redecl-add-after-load.cpp Thu Mar 19 21:17:21 2015
@@ -29,7 +29,7 @@ struct D {
static constexpr int function(); // expected-note {{here}}
};
typedef D::A DB;
-constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{subexpression}} expected-note {{undefined}}
+constexpr int D_test(bool b) { return b ? D::variable : D::function(); } // expected-note {{undefined}}
#endif
@import redecl_add_after_load;
@@ -54,6 +54,6 @@ constexpr int merged_struct_variable_tes
constexpr int merged_struct_function_test = D_test(false);
#ifndef IMPORT_DECLS
// expected-error at -4 {{incomplete}}
-// expected-error at -4 {{constant}} expected-note at -4 {{in call to}}
+// @-4: definition of D::variable must be emitted, so it gets imported eagerly
// expected-error at -4 {{constant}} expected-note at -4 {{in call to}}
#endif
More information about the cfe-commits
mailing list