[clang-tools-extra] r327717 - [clangd] Handle multiple callbacks from Sema's completion

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Mar 16 08:23:44 2018
New Revision: 327717

URL: http://llvm.org/viewvc/llvm-project?rev=327717&view=rev
Log:
[clangd] Handle multiple callbacks from Sema's completion

Summary:
When parser backtracks, we might receive multiple code completion
callbacks.
Previously we had a failing assertion there, now we take first results
and hope they are good enough.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, ioeric, cfe-commits

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

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=327717&r1=327716&r2=327717&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Mar 16 08:23:44 2018
@@ -450,8 +450,15 @@ struct CompletionRecorder : public CodeC
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *InResults,
                                   unsigned NumResults) override final {
+    if (CCSema) {
+      log(llvm::formatv(
+          "Multiple code complete callbacks (parser backtracked?). "
+          "Dropping results from context {0}, keeping results from {1}.",
+          getCompletionKindString(this->CCContext.getKind()),
+          getCompletionKindString(Context.getKind())));
+      return;
+    }
     // Record the completion context.
-    assert(!CCSema && "ProcessCodeCompleteResults called multiple times!");
     CCSema = &S;
     CCContext = Context;
 

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=327717&r1=327716&r2=327717&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Mar 16 08:23:44 2018
@@ -608,6 +608,43 @@ TEST(CodeCompleteTest, NoColonColonAtThe
   EXPECT_THAT(Results.items, Not(Contains(Labeled("clang::"))));
 }
 
+TEST(CompletionTest, BacktrackCrashes) {
+  // Sema calls code completion callbacks twice in these cases.
+  auto Results = completions(R"cpp(
+      namespace ns {
+      struct FooBarBaz {};
+      } // namespace ns
+
+     int foo(ns::FooBar^
+  )cpp");
+
+  EXPECT_THAT(Results.items, ElementsAre(Labeled("FooBarBaz")));
+
+  // Check we don't crash in that case too.
+  completions(R"cpp(
+    struct FooBarBaz {};
+    void test() {
+      if (FooBarBaz * x^) {}
+    }
+)cpp");
+}
+
+TEST(CompletionTest, CompleteInExcludedPPBranch) {
+  auto Results = completions(R"cpp(
+    int bar(int param_in_bar) {
+    }
+
+    int foo(int param_in_foo) {
+#if 0
+  par^
+#endif
+    }
+)cpp");
+
+  EXPECT_THAT(Results.items, Contains(Labeled("param_in_foo")));
+  EXPECT_THAT(Results.items, Not(Contains(Labeled("param_in_bar"))));
+}
+
 SignatureHelp signatures(StringRef Text) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;