Skip to content

Commit 091b1ef

Browse files
sam-mccallsam-mccall
committedJan 16, 2018
Ensure code complete with !LoadExternal sees all local decls.
Summary: noload_lookups() was too lazy: in addition to avoiding external decls, it avoided populating the lazy lookup structure for internal decls. This is the right behavior for the existing callsite in ASTDumper, but I think it's not a very useful default, so we populate it by default. While here: - remove an unused test file accidentally added in r322371. - remove lookups_begin()/lookups_end() in favor of lookups().begin(), which is more common and more efficient. Reviewers: ilya-biryukov Subscribers: cfe-commits, rsmith Differential Revision: https://reviews.llvm.org/D42077 llvm-svn: 322548
1 parent 2c3849a commit 091b1ef

File tree

7 files changed

+35
-40
lines changed

7 files changed

+35
-40
lines changed
 

‎clang/include/clang/AST/DeclBase.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -1823,7 +1823,9 @@ class DeclContext {
18231823
using lookups_range = llvm::iterator_range<all_lookups_iterator>;
18241824

18251825
lookups_range lookups() const;
1826-
lookups_range noload_lookups() const;
1826+
// Like lookups(), but avoids loading external declarations.
1827+
// If PreserveInternalState, avoids building lookup data structures too.
1828+
lookups_range noload_lookups(bool PreserveInternalState) const;
18271829

18281830
/// \brief Iterators over all possible lookups within this context.
18291831
all_lookups_iterator lookups_begin() const;
@@ -1943,6 +1945,7 @@ class DeclContext {
19431945

19441946
StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;
19451947

1948+
void loadLazyLocalLexicalLookups();
19461949
void buildLookupImpl(DeclContext *DCtx, bool Internal);
19471950
void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
19481951
bool Rediscoverable);

‎clang/include/clang/AST/DeclLookups.h

+4-19
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,11 @@ inline DeclContext::lookups_range DeclContext::lookups() const {
8686
return lookups_range(all_lookups_iterator(), all_lookups_iterator());
8787
}
8888

89-
inline DeclContext::all_lookups_iterator DeclContext::lookups_begin() const {
90-
return lookups().begin();
91-
}
92-
93-
inline DeclContext::all_lookups_iterator DeclContext::lookups_end() const {
94-
return lookups().end();
95-
}
96-
97-
inline DeclContext::lookups_range DeclContext::noload_lookups() const {
89+
inline DeclContext::lookups_range
90+
DeclContext::noload_lookups(bool PreserveInternalState) const {
9891
DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
92+
if (!PreserveInternalState)
93+
Primary->loadLazyLocalLexicalLookups();
9994
if (StoredDeclsMap *Map = Primary->getLookupPtr())
10095
return lookups_range(all_lookups_iterator(Map->begin(), Map->end()),
10196
all_lookups_iterator(Map->end(), Map->end()));
@@ -105,16 +100,6 @@ inline DeclContext::lookups_range DeclContext::noload_lookups() const {
105100
return lookups_range(all_lookups_iterator(), all_lookups_iterator());
106101
}
107102

108-
inline
109-
DeclContext::all_lookups_iterator DeclContext::noload_lookups_begin() const {
110-
return noload_lookups().begin();
111-
}
112-
113-
inline
114-
DeclContext::all_lookups_iterator DeclContext::noload_lookups_end() const {
115-
return noload_lookups().end();
116-
}
117-
118103
} // namespace clang
119104

120105
#endif // LLVM_CLANG_AST_DECLLOOKUPS_H

‎clang/lib/AST/ASTDumper.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -809,11 +809,10 @@ void ASTDumper::dumpLookups(const DeclContext *DC, bool DumpDecls) {
809809

810810
bool HasUndeserializedLookups = Primary->hasExternalVisibleStorage();
811811

812-
for (auto I = Deserialize ? Primary->lookups_begin()
813-
: Primary->noload_lookups_begin(),
814-
E = Deserialize ? Primary->lookups_end()
815-
: Primary->noload_lookups_end();
816-
I != E; ++I) {
812+
auto Range = Deserialize
813+
? Primary->lookups()
814+
: Primary->noload_lookups(/*PreserveInternalState=*/true);
815+
for (auto I = Range.begin(), E = Range.end(); I != E; ++I) {
817816
DeclarationName Name = I.getLookupName();
818817
DeclContextLookupResult R = *I;
819818

‎clang/lib/AST/DeclBase.cpp

+14-11
Original file line numberDiff line numberDiff line change
@@ -1588,17 +1588,7 @@ DeclContext::noload_lookup(DeclarationName Name) {
15881588
if (PrimaryContext != this)
15891589
return PrimaryContext->noload_lookup(Name);
15901590

1591-
// If we have any lazy lexical declarations not in our lookup map, add them
1592-
// now. Don't import any external declarations, not even if we know we have
1593-
// some missing from the external visible lookups.
1594-
if (HasLazyLocalLexicalLookups) {
1595-
SmallVector<DeclContext *, 2> Contexts;
1596-
collectAllContexts(Contexts);
1597-
for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
1598-
buildLookupImpl(Contexts[I], hasExternalVisibleStorage());
1599-
HasLazyLocalLexicalLookups = false;
1600-
}
1601-
1591+
loadLazyLocalLexicalLookups();
16021592
StoredDeclsMap *Map = LookupPtr;
16031593
if (!Map)
16041594
return lookup_result();
@@ -1608,6 +1598,19 @@ DeclContext::noload_lookup(DeclarationName Name) {
16081598
: lookup_result();
16091599
}
16101600

1601+
// If we have any lazy lexical declarations not in our lookup map, add them
1602+
// now. Don't import any external declarations, not even if we know we have
1603+
// some missing from the external visible lookups.
1604+
void DeclContext::loadLazyLocalLexicalLookups() {
1605+
if (HasLazyLocalLexicalLookups) {
1606+
SmallVector<DeclContext *, 2> Contexts;
1607+
collectAllContexts(Contexts);
1608+
for (unsigned I = 0, N = Contexts.size(); I != N; ++I)
1609+
buildLookupImpl(Contexts[I], hasExternalVisibleStorage());
1610+
HasLazyLocalLexicalLookups = false;
1611+
}
1612+
}
1613+
16111614
void DeclContext::localUncachedLookup(DeclarationName Name,
16121615
SmallVectorImpl<NamedDecl *> &Results) {
16131616
Results.clear();

‎clang/lib/Sema/SemaLookup.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -3543,7 +3543,8 @@ static void LookupVisibleDecls(DeclContext *Ctx, LookupResult &Result,
35433543

35443544
// Enumerate all of the results in this context.
35453545
for (DeclContextLookupResult R :
3546-
LoadExternal ? Ctx->lookups() : Ctx->noload_lookups()) {
3546+
LoadExternal ? Ctx->lookups()
3547+
: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
35473548
for (auto *D : R) {
35483549
if (auto *ND = Result.getAcceptableDecl(D)) {
35493550
Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);

‎clang/test/Index/complete-pch-skip.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,10 @@ int main() { return ns:: }
1616
// SKIP-PCH: {TypedText bar}
1717
// SKIP-PCH-NOT: foo
1818

19+
// Verify that with *no* preamble (no -include flag) we still get local results.
20+
// SkipPreamble used to break this, by making lookup *too* lazy.
21+
// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 %s | FileCheck -check-prefix=NO-PCH %s
22+
// NO-PCH-NOT: foo
23+
// NO-PCH: {TypedText bar}
24+
// NO-PCH-NOT: foo
25+

‎clang/test/Index/complete-pch-skip.h

-3
This file was deleted.

0 commit comments

Comments
 (0)
Please sign in to comment.