ioeric created this revision.
ioeric added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, klimek.
Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.
For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".
void f(const char*, int);
int x;
void main() {
F(::^);
}
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47183
Files:
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -673,16 +673,17 @@
}
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")));
+ // Sema calls code completion callbacks twice in these cases.
+ // FIXME: deduplicate identical results.
+ EXPECT_THAT(Results.items, Contains(Labeled("FooBarBaz")));
// Check we don't crash in that case too.
completions(R"cpp(
@@ -693,6 +694,26 @@
)cpp");
}
+TEST(CompletionTest, CompleteInMacroWithStringification) {
+ auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+} // namespace ns
+
+int f(int input_num) {
+ F(ns::^)
+}
+ }
+)cpp");
+
+ EXPECT_THAT(Results.items,
+ UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
TEST(CompletionTest, CompleteInExcludedPPBranch) {
auto Results = completions(R"cpp(
int bar(int param_in_bar) {
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -431,14 +431,8 @@
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;
- }
+ log("Processing completion results in context " +
+ getCompletionKindString(Context.getKind()));
// Record the completion context.
CCSema = &S;
CCContext = Context;
@@ -460,6 +454,8 @@
continue;
// We choose to never append '::' to completion results in clangd.
Result.StartsNestedNameSpecifier = false;
+ // FIXME: the same result can be added multiple times as the callback can
+ // be called more than once. We may want to deduplicate identical results.
Results.push_back(Result);
}
ResultsCallback();
@@ -725,6 +721,8 @@
return false;
}
Action.EndSourceFile();
+ if (Includes)
+ Includes->reset(); // Make sure this doesn't out-live Clang.
return true;
}
@@ -878,8 +876,10 @@
assert(Includes && "Includes is not set");
// If preprocessor was run, inclusions from preprocessor callback should
// already be added to Inclusions.
- Output = runWithSema();
- Includes.reset(); // Make sure this doesn't out-live Clang.
+ auto Items = runWithSema();
+ Output.items.insert(Output.items.end(),
+ std::make_move_iterator(Items.begin()),
+ std::make_move_iterator(Items.end()));
SPAN_ATTACH(Tracer, "sema_completion_kind",
getCompletionKindString(Recorder->CCContext.getKind()));
});
@@ -898,15 +898,16 @@
NSema, NIndex, NBoth, Output.items.size(),
Output.isIncomplete ? " (incomplete)" : ""));
assert(!Opts.Limit || Output.items.size() <= Opts.Limit);
+ Output.isIncomplete = Incomplete;
// We don't assert that isIncomplete means we hit a limit.
// Indexes may choose to impose their own limits even if we don't have one.
return Output;
}
private:
// This is called by run() once Sema code completion is done, but before the
// Sema data structures are torn down. It does all the real work.
- CompletionList runWithSema() {
+ std::vector<CompletionItem> runWithSema() {
Filter = FuzzyMatcher(
Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
// Sema provides the needed context to query the index.
@@ -918,10 +919,9 @@
// Merge Sema and Index results, score them, and pick the winners.
auto Top = mergeResults(Recorder->Results, IndexResults);
// Convert the results to the desired LSP structs.
- CompletionList Output;
+ std::vector<CompletionItem> Output;
for (auto &C : Top)
- Output.items.push_back(toCompletionItem(C.first, C.second));
- Output.isIncomplete = Incomplete;
+ Output.push_back(toCompletionItem(C.first, C.second));
return Output;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
