[clang-tools-extra] r342230 - [clangd] Don't override the preamble while completing inside it, it doesn't work.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Sep 14 05:36:06 2018
New Revision: 342230

URL: http://llvm.org/viewvc/llvm-project?rev=342230&view=rev
Log:
[clangd] Don't override the preamble while completing inside it, it doesn't work.

Summary:
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.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D52071

Modified:
    clang-tools-extra/trunk/clangd/CodeComplete.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=342230&r1=342229&r2=342230&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Sep 14 05:36:06 2018
@@ -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 @@ bool semaCodeComplete(std::unique_ptr<Co
   // 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;

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=342230&r1=342229&r2=342230&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Sep 14 05:36:06 2018
@@ -657,6 +657,22 @@ TEST(CompletionTest, IndexSuppressesPrea
               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;