sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric.
To stay fast, enable single-file-mode instead. This is fine since completions in the preamble are simple. The net effect for now is to suppress the spurious TopLevel completions when completing inside the preamble. Once Sema has include directive completion, this will be more important. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52071 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -657,6 +657,22 @@ UnorderedElementsAre(Named("local"), Named("preamble"))); } +// This verifies that we get normal preprocessor completions in the preamble. +// This is a regression test for an old bug: if we override the preamble and +// try to complete inside it, clang kicks our completion point just outside the +// preamble, resulting in always getting top-level completions. +TEST(CompletionTest, CompletionInPreamble) { + EXPECT_THAT(completions(R"cpp( + #ifnd^ef FOO_H_ + #define BAR_H_ + #include <bar.h> + int foo() {} + #endif + )cpp") + .Completions, + ElementsAre(Named("ifndef"))); +}; + TEST(CompletionTest, DynamicIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Index/USRGeneration.h" +#include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" @@ -1053,11 +1054,19 @@ // We reuse the preamble whether it's valid or not. This is a // correctness/performance tradeoff: building without a preamble is slow, and // completion is latency-sensitive. + // However, if we're completing *inside* the preamble section of the draft, + // overriding the preamble will break sema completion. Fortunately we can just + // skip all includes in this case; these completions are really simple. + bool CompletingInPreamble = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size > + *Offset; // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. auto Clang = prepareCompilerInstance( - std::move(CI), Input.Preamble, std::move(ContentsBuffer), - std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer); + std::move(CI), CompletingInPreamble ? nullptr : Input.Preamble, + std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), + DummyDiagsConsumer); + Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble; Clang->setCodeCompletionConsumer(Consumer.release()); SyntaxOnlyAction Action;
Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -657,6 +657,22 @@ UnorderedElementsAre(Named("local"), Named("preamble"))); } +// This verifies that we get normal preprocessor completions in the preamble. +// This is a regression test for an old bug: if we override the preamble and +// try to complete inside it, clang kicks our completion point just outside the +// preamble, resulting in always getting top-level completions. +TEST(CompletionTest, CompletionInPreamble) { + EXPECT_THAT(completions(R"cpp( + #ifnd^ef FOO_H_ + #define BAR_H_ + #include <bar.h> + int foo() {} + #endif + )cpp") + .Completions, + ElementsAre(Named("ifndef"))); +}; + TEST(CompletionTest, DynamicIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Index/USRGeneration.h" +#include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" @@ -1053,11 +1054,19 @@ // We reuse the preamble whether it's valid or not. This is a // correctness/performance tradeoff: building without a preamble is slow, and // completion is latency-sensitive. + // However, if we're completing *inside* the preamble section of the draft, + // overriding the preamble will break sema completion. Fortunately we can just + // skip all includes in this case; these completions are really simple. + bool CompletingInPreamble = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size > + *Offset; // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. auto Clang = prepareCompilerInstance( - std::move(CI), Input.Preamble, std::move(ContentsBuffer), - std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer); + std::move(CI), CompletingInPreamble ? nullptr : Input.Preamble, + std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), + DummyDiagsConsumer); + Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble; Clang->setCodeCompletionConsumer(Consumer.release()); SyntaxOnlyAction Action;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits