Author: sammccall Date: Fri Jan 12 10:30:08 2018 New Revision: 322387 URL: http://llvm.org/viewvc/llvm-project?rev=322387&view=rev Log: [clangd] Code completion uses Sema for NS-level things in the current file.
Summary: To stay fast, it avoids deserializing anything outside the current file, by disabling the LoadExternal code completion option added in r322377, when the index is enabled. Reviewers: hokein Subscribers: klimek, ilya-biryukov, cfe-commits Differential Revision: https://reviews.llvm.org/D41996 Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/index/FileIndex.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322387&r1=322386&r2=322387&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jan 12 10:30:08 2018 @@ -642,8 +642,10 @@ clang::CodeCompleteOptions CodeCompleteO Result.IncludeGlobals = IncludeGlobals; Result.IncludeBriefComments = IncludeBriefComments; - // Enable index-based code completion when Index is provided. - Result.IncludeNamespaceLevelDecls = !Index; + // When an is used, Sema is responsible for completing the main file, + // the index can provide results from the preamble. + // Tell Sema not to deserialize the preamble to look for results. + Result.LoadExternal = !Index; return Result; } Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322387&r1=322386&r2=322387&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Fri Jan 12 10:30:08 2018 @@ -20,6 +20,8 @@ std::unique_ptr<SymbolSlab> indexAST(AST std::shared_ptr<Preprocessor> PP, llvm::ArrayRef<const Decl *> Decls) { SymbolCollector::Options CollectorOpts; + // Code completion gets main-file results from Sema. + // But we leave this option on because features like go-to-definition want it. CollectorOpts.IndexMainFiles = true; auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts)); Collector->setPreprocessor(std::move(PP)); Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=322387&r1=322386&r2=322387&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jan 12 10:30:08 2018 @@ -57,6 +57,7 @@ using ::testing::Contains; using ::testing::Each; using ::testing::ElementsAre; using ::testing::Not; +using ::testing::UnorderedElementsAre; class IgnoreDiagnostics : public DiagnosticsConsumer { void @@ -104,7 +105,7 @@ CompletionList completions(StringRef Tex /*StorePreamblesInMemory=*/true); auto File = getVirtualTestFilePath("foo.cpp"); Annotations Test(Text); - Server.addDocument(Context::empty(), File, Test.code()); + Server.addDocument(Context::empty(), File, Test.code()).wait(); auto CompletionList = Server.codeComplete(Context::empty(), File, Test.point(), Opts) .get() @@ -506,11 +507,11 @@ TEST(CompletionTest, NoIndex) { Opts.Index = nullptr; auto Results = completions(R"cpp( - namespace ns { class No {}; } + namespace ns { class Local {}; } void f() { ns::^ } )cpp", Opts); - EXPECT_THAT(Results.items, Has("No")); + EXPECT_THAT(Results.items, Has("Local")); } TEST(CompletionTest, StaticAndDynamicIndex) { @@ -538,13 +539,13 @@ TEST(CompletionTest, SimpleIndexBased) { Opts.Index = I.get(); auto Results = completions(R"cpp( - namespace ns { class No {}; } + namespace ns { int local; } void f() { ns::^ } )cpp", Opts); EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class)); EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function)); - EXPECT_THAT(Results.items, Not(Has("No"))); + EXPECT_THAT(Results.items, Has("local")); } TEST(CompletionTest, IndexBasedWithFilter) { @@ -585,6 +586,41 @@ TEST(CompletionTest, FullyQualifiedScope EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class)); } +TEST(CompletionTest, IndexSuppressesPreambleCompletions) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + /*StorePreamblesInMemory=*/true); + + FS.Files[getVirtualTestFilePath("bar.h")] = + R"cpp(namespace ns { int preamble; })cpp"; + auto File = getVirtualTestFilePath("foo.cpp"); + Annotations Test(R"cpp( + #include "bar.h" + namespace ns { int local; } + void f() { ns::^ } + )cpp"); + Server.addDocument(Context::empty(), File, Test.code()).wait(); + clangd::CodeCompleteOptions Opts = {}; + + auto WithoutIndex = + Server.codeComplete(Context::empty(), File, Test.point(), Opts) + .get() + .second.Value; + EXPECT_THAT(WithoutIndex.items, + UnorderedElementsAre(Named("local"), Named("preamble"))); + + auto I = simpleIndexFromSymbols({{"ns::index", index::SymbolKind::Variable}}); + Opts.Index = I.get(); + auto WithIndex = + Server.codeComplete(Context::empty(), File, Test.point(), Opts) + .get() + .second.Value; + EXPECT_THAT(WithIndex.items, + UnorderedElementsAre(Named("local"), Named("index"))); +} + TEST(CompletionTest, ASTIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits