diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 7392529a00d4..f4f6c39e2929 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -586,6 +586,19 @@ struct CompletionRecorder : public CodeCompleteConsumer { 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. diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp index cc35c4f62320..86a5f415163e 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp @@ -787,20 +787,24 @@ int f(int input_num) { 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 @@ TEST(CompletionTest, Render) { 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