Author: sammccall Date: Tue Jan 16 04:33:46 2018 New Revision: 322548 URL: http://llvm.org/viewvc/llvm-project?rev=322548&view=rev Log: 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 Removed: cfe/trunk/test/Index/complete-pch-skip.h Modified: cfe/trunk/include/clang/AST/DeclBase.h cfe/trunk/include/clang/AST/DeclLookups.h cfe/trunk/lib/AST/ASTDumper.cpp cfe/trunk/lib/AST/DeclBase.cpp cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/test/Index/complete-pch-skip.cpp Modified: cfe/trunk/include/clang/AST/DeclBase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclBase.h (original) +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Jan 16 04:33:46 2018 @@ -1823,7 +1823,9 @@ public: using lookups_range = llvm::iterator_range<all_lookups_iterator>; lookups_range lookups() const; - lookups_range noload_lookups() const; + // Like lookups(), but avoids loading external declarations. + // If PreserveInternalState, avoids building lookup data structures too. + lookups_range noload_lookups(bool PreserveInternalState) const; /// \brief Iterators over all possible lookups within this context. all_lookups_iterator lookups_begin() const; @@ -1943,6 +1945,7 @@ private: StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const; + void loadLazyLocalLexicalLookups(); void buildLookupImpl(DeclContext *DCtx, bool Internal); void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal, bool Rediscoverable); Modified: cfe/trunk/include/clang/AST/DeclLookups.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclLookups.h?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclLookups.h (original) +++ cfe/trunk/include/clang/AST/DeclLookups.h Tue Jan 16 04:33:46 2018 @@ -86,16 +86,11 @@ inline DeclContext::lookups_range DeclCo return lookups_range(all_lookups_iterator(), all_lookups_iterator()); } -inline DeclContext::all_lookups_iterator DeclContext::lookups_begin() const { - return lookups().begin(); -} - -inline DeclContext::all_lookups_iterator DeclContext::lookups_end() const { - return lookups().end(); -} - -inline DeclContext::lookups_range DeclContext::noload_lookups() const { +inline DeclContext::lookups_range +DeclContext::noload_lookups(bool PreserveInternalState) const { DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext(); + if (!PreserveInternalState) + Primary->loadLazyLocalLexicalLookups(); if (StoredDeclsMap *Map = Primary->getLookupPtr()) return lookups_range(all_lookups_iterator(Map->begin(), Map->end()), all_lookups_iterator(Map->end(), Map->end())); @@ -105,16 +100,6 @@ inline DeclContext::lookups_range DeclCo return lookups_range(all_lookups_iterator(), all_lookups_iterator()); } -inline -DeclContext::all_lookups_iterator DeclContext::noload_lookups_begin() const { - return noload_lookups().begin(); -} - -inline -DeclContext::all_lookups_iterator DeclContext::noload_lookups_end() const { - return noload_lookups().end(); -} - } // namespace clang #endif // LLVM_CLANG_AST_DECLLOOKUPS_H Modified: cfe/trunk/lib/AST/ASTDumper.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTDumper.cpp (original) +++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jan 16 04:33:46 2018 @@ -809,11 +809,10 @@ void ASTDumper::dumpLookups(const DeclCo bool HasUndeserializedLookups = Primary->hasExternalVisibleStorage(); - for (auto I = Deserialize ? Primary->lookups_begin() - : Primary->noload_lookups_begin(), - E = Deserialize ? Primary->lookups_end() - : Primary->noload_lookups_end(); - I != E; ++I) { + auto Range = Deserialize + ? Primary->lookups() + : Primary->noload_lookups(/*PreserveInternalState=*/true); + for (auto I = Range.begin(), E = Range.end(); I != E; ++I) { DeclarationName Name = I.getLookupName(); DeclContextLookupResult R = *I; Modified: cfe/trunk/lib/AST/DeclBase.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/lib/AST/DeclBase.cpp (original) +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Jan 16 04:33:46 2018 @@ -1588,17 +1588,7 @@ DeclContext::noload_lookup(DeclarationNa if (PrimaryContext != this) return PrimaryContext->noload_lookup(Name); - // If we have any lazy lexical declarations not in our lookup map, add them - // now. Don't import any external declarations, not even if we know we have - // some missing from the external visible lookups. - if (HasLazyLocalLexicalLookups) { - SmallVector<DeclContext *, 2> Contexts; - collectAllContexts(Contexts); - for (unsigned I = 0, N = Contexts.size(); I != N; ++I) - buildLookupImpl(Contexts[I], hasExternalVisibleStorage()); - HasLazyLocalLexicalLookups = false; - } - + loadLazyLocalLexicalLookups(); StoredDeclsMap *Map = LookupPtr; if (!Map) return lookup_result(); @@ -1608,6 +1598,19 @@ DeclContext::noload_lookup(DeclarationNa : lookup_result(); } +// If we have any lazy lexical declarations not in our lookup map, add them +// now. Don't import any external declarations, not even if we know we have +// some missing from the external visible lookups. +void DeclContext::loadLazyLocalLexicalLookups() { + if (HasLazyLocalLexicalLookups) { + SmallVector<DeclContext *, 2> Contexts; + collectAllContexts(Contexts); + for (unsigned I = 0, N = Contexts.size(); I != N; ++I) + buildLookupImpl(Contexts[I], hasExternalVisibleStorage()); + HasLazyLocalLexicalLookups = false; + } +} + void DeclContext::localUncachedLookup(DeclarationName Name, SmallVectorImpl<NamedDecl *> &Results) { Results.clear(); Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Jan 16 04:33:46 2018 @@ -3543,7 +3543,8 @@ static void LookupVisibleDecls(DeclConte // Enumerate all of the results in this context. for (DeclContextLookupResult R : - LoadExternal ? Ctx->lookups() : Ctx->noload_lookups()) { + LoadExternal ? Ctx->lookups() + : Ctx->noload_lookups(/*PreserveInternalState=*/false)) { for (auto *D : R) { if (auto *ND = Result.getAcceptableDecl(D)) { Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass); Modified: cfe/trunk/test/Index/complete-pch-skip.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-pch-skip.cpp?rev=322548&r1=322547&r2=322548&view=diff ============================================================================== --- cfe/trunk/test/Index/complete-pch-skip.cpp (original) +++ cfe/trunk/test/Index/complete-pch-skip.cpp Tue Jan 16 04:33:46 2018 @@ -16,3 +16,10 @@ int main() { return ns:: } // SKIP-PCH: {TypedText bar} // SKIP-PCH-NOT: foo +// Verify that with *no* preamble (no -include flag) we still get local results. +// SkipPreamble used to break this, by making lookup *too* lazy. +// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 %s | FileCheck -check-prefix=NO-PCH %s +// NO-PCH-NOT: foo +// NO-PCH: {TypedText bar} +// NO-PCH-NOT: foo + Removed: cfe/trunk/test/Index/complete-pch-skip.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-pch-skip.h?rev=322547&view=auto ============================================================================== --- cfe/trunk/test/Index/complete-pch-skip.h (original) +++ cfe/trunk/test/Index/complete-pch-skip.h (removed) @@ -1,3 +0,0 @@ -namespace ns { -int foo; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits