[PATCH] D49175: [clangd] Ignore sema code complete callback with recovery context.

Eric Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 11 06:20:28 PDT 2018


This revision was automatically updated to reflect the committed changes.
Closed by commit rL336801: [clangd] Ignore sema code complete callback with recovery context. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D49175

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp


Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -586,6 +586,19 @@
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *InResults,
                                   unsigned NumResults) override final {
+    // Results from recovery mode are generally useless, and the callback after
+    // recovery (if any) is usually more interesting. To make sure we handle the
+    // future callback from sema, we just ignore all callbacks in recovery mode,
+    // as taking only results from recovery mode results in poor completion
+    // results.
+    // FIXME: in case there is no future sema completion callback after the
+    // recovery mode, we might still want to provide some results (e.g. trivial
+    // identifier-based completion).
+    if (Context.getKind() == CodeCompletionContext::CCC_Recovery) {
+      log("Code complete: Ignoring sema code complete callback with Recovery "
+          "context.");
+      return;
+    }
     // If a callback is called without any sema result and the context does not
     // support index-based completion, we simply skip it to give way to
     // potential future callbacks with results.
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -787,20 +787,24 @@
   EXPECT_THAT(Results.Completions, Contains(Named("X")));
 }
 
-TEST(CompletionTest, CompleteInExcludedPPBranch) {
+TEST(CompletionTest, IgnoreCompleteInExcludedPPBranchWithRecoveryContext) {
   auto Results = completions(R"cpp(
     int bar(int param_in_bar) {
     }
 
     int foo(int param_in_foo) {
 #if 0
+  // In recorvery mode, "param_in_foo" will also be suggested among many other
+  // unrelated symbols; however, this is really a special case where this works.
+  // If the #if block is outside of the function, "param_in_foo" is still
+  // suggested, but "bar" and "foo" are missing. So the recovery mode doesn't
+  // really provide useful results in excluded branches.
   par^
 #endif
     }
 )cpp");
 
-  EXPECT_THAT(Results.Completions, Contains(Labeled("param_in_foo")));
-  EXPECT_THAT(Results.Completions, Not(Contains(Labeled("param_in_bar"))));
+  EXPECT_TRUE(Results.Completions.empty());
 }
 
 SignatureHelp signatures(StringRef Text) {
@@ -1293,6 +1297,18 @@
   EXPECT_EQ(R.detail, "[2 overloads]\n\"foo.h\"");
 }
 
+TEST(CompletionTest, IgnoreRecoveryResults) {
+  auto Results = completions(
+      R"cpp(
+          namespace ns { int NotRecovered() { return 0; } }
+          void f() {
+            // Sema enters recovery mode first and then normal mode.
+            if (auto x = ns::NotRecover^)
+          }
+      )cpp");
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("NotRecovered")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49175.154984.patch
Type: text/x-patch
Size: 3204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180711/fba54d86/attachment.bin>


More information about the llvm-commits mailing list