diff options
| -rw-r--r-- | clang-tools-extra/clangd/CodeComplete.cpp | 67 | ||||
| -rw-r--r-- | clang-tools-extra/clangd/Quality.cpp | 12 | ||||
| -rw-r--r-- | clang-tools-extra/clangd/Quality.h | 10 | ||||
| -rw-r--r-- | clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp | 22 |
4 files changed, 100 insertions, 11 deletions
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 11d41f0ea7a..12a3d5f48f5 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -129,16 +129,16 @@ toCompletionItemKind(CodeCompletionResult::ResultKind ResKind, /// Get the optional chunk as a string. This function is possibly recursive. /// /// The parameter info for each parameter is appended to the Parameters. -std::string -getOptionalParameters(const CodeCompletionString &CCS, - std::vector<ParameterInformation> &Parameters) { +std::string getOptionalParameters(const CodeCompletionString &CCS, + std::vector<ParameterInformation> &Parameters, + SignatureQualitySignals &Signal) { std::string Result; for (const auto &Chunk : CCS) { switch (Chunk.Kind) { case CodeCompletionString::CK_Optional: assert(Chunk.Optional && "Expected the optional code completion string to be non-null."); - Result += getOptionalParameters(*Chunk.Optional, Parameters); + Result += getOptionalParameters(*Chunk.Optional, Parameters, Signal); break; case CodeCompletionString::CK_VerticalSpace: break; @@ -154,6 +154,8 @@ getOptionalParameters(const CodeCompletionString &CCS, ParameterInformation Info; Info.label = Chunk.Text; Parameters.push_back(std::move(Info)); + Signal.ContainsActiveParameter = true; + Signal.NumberOfOptionalParameters++; break; } default: @@ -685,6 +687,9 @@ private: llvm::unique_function<void()> ResultsCallback; }; +using ScoredSignature = + std::pair<SignatureQualitySignals, SignatureInformation>; + class SignatureHelpCollector final : public CodeCompleteConsumer { public: @@ -698,7 +703,9 @@ public: void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg, OverloadCandidate *Candidates, unsigned NumCandidates) override { + std::vector<ScoredSignature> ScoredSignatures; SigHelp.signatures.reserve(NumCandidates); + ScoredSignatures.reserve(NumCandidates); // FIXME(rwols): How can we determine the "active overload candidate"? // Right now the overloaded candidates seem to be provided in a "best fit" // order, so I'm not too worried about this. @@ -712,11 +719,45 @@ public: CurrentArg, S, *Allocator, CCTUInfo, true); assert(CCS && "Expected the CodeCompletionString to be non-null"); // FIXME: for headers, we need to get a comment from the index. - SigHelp.signatures.push_back(processOverloadCandidate( + ScoredSignatures.push_back(processOverloadCandidate( Candidate, *CCS, getParameterDocComment(S.getASTContext(), Candidate, CurrentArg, /*CommentsFromHeaders=*/false))); } + std::sort(ScoredSignatures.begin(), ScoredSignatures.end(), + [](const ScoredSignature &L, const ScoredSignature &R) { + // Ordering follows: + // - Less number of parameters is better. + // - Function is better than FunctionType which is better than + // Function Template. + // - High score is better. + // - Shorter signature is better. + // - Alphebatically smaller is better. + if (L.first.NumberOfParameters != R.first.NumberOfParameters) + return L.first.NumberOfParameters < + R.first.NumberOfParameters; + if (L.first.NumberOfOptionalParameters != + R.first.NumberOfOptionalParameters) + return L.first.NumberOfOptionalParameters < + R.first.NumberOfOptionalParameters; + if (L.first.Kind != R.first.Kind) { + using OC = CodeCompleteConsumer::OverloadCandidate; + switch (L.first.Kind) { + case OC::CK_Function: + return true; + case OC::CK_FunctionType: + return R.first.Kind != OC::CK_Function; + case OC::CK_FunctionTemplate: + return false; + } + llvm_unreachable("Unknown overload candidate type."); + } + if (L.second.label.size() != R.second.label.size()) + return L.second.label.size() < R.second.label.size(); + return L.second.label < R.second.label; + }); + for (const auto &SS : ScoredSignatures) + SigHelp.signatures.push_back(SS.second); } GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; } @@ -726,14 +767,15 @@ public: private: // FIXME(ioeric): consider moving CodeCompletionString logic here to // CompletionString.h. - SignatureInformation - processOverloadCandidate(const OverloadCandidate &Candidate, - const CodeCompletionString &CCS, - llvm::StringRef DocComment) const { + ScoredSignature processOverloadCandidate(const OverloadCandidate &Candidate, + const CodeCompletionString &CCS, + llvm::StringRef DocComment) const { SignatureInformation Result; + SignatureQualitySignals Signal; const char *ReturnType = nullptr; Result.documentation = formatDocumentation(CCS, DocComment); + Signal.Kind = Candidate.getKind(); for (const auto &Chunk : CCS) { switch (Chunk.Kind) { @@ -755,6 +797,8 @@ private: ParameterInformation Info; Info.label = Chunk.Text; Result.parameters.push_back(std::move(Info)); + Signal.NumberOfParameters++; + Signal.ContainsActiveParameter = true; break; } case CodeCompletionString::CK_Optional: { @@ -762,7 +806,7 @@ private: assert(Chunk.Optional && "Expected the optional code completion string to be non-null."); Result.label += - getOptionalParameters(*Chunk.Optional, Result.parameters); + getOptionalParameters(*Chunk.Optional, Result.parameters, Signal); break; } case CodeCompletionString::CK_VerticalSpace: @@ -776,7 +820,8 @@ private: Result.label += " -> "; Result.label += ReturnType; } - return Result; + dlog("Signal for {0}: {1}", Result, Signal); + return {Signal, Result}; } SignatureHelp &SigHelp; diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp index 8cf794b0fe8..f96b50e2702 100644 --- a/clang-tools-extra/clangd/Quality.cpp +++ b/clang-tools-extra/clangd/Quality.cpp @@ -401,5 +401,17 @@ std::string sortText(float Score, llvm::StringRef Name) { return S; } +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, + const SignatureQualitySignals &S) { + OS << formatv("=== Signature Quality:\n"); + OS << formatv("\tNumber of parameters: {0}\n", S.NumberOfParameters); + OS << formatv("\tNumber of optional parameters: {0}\n", + S.NumberOfOptionalParameters); + OS << formatv("\tContains active parameter: {0}\n", + S.ContainsActiveParameter); + OS << formatv("\tKind: {0}\n", S.Kind); + return OS; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Quality.h b/clang-tools-extra/clangd/Quality.h index 54778b9fa76..86e21963dd5 100644 --- a/clang-tools-extra/clangd/Quality.h +++ b/clang-tools-extra/clangd/Quality.h @@ -163,6 +163,16 @@ private: /// LSP. (The highest score compares smallest so it sorts at the top). std::string sortText(float Score, llvm::StringRef Tiebreak = ""); +struct SignatureQualitySignals { + uint32_t NumberOfParameters = 0; + uint32_t NumberOfOptionalParameters = 0; + bool ContainsActiveParameter = false; + CodeCompleteConsumer::OverloadCandidate::CandidateKind Kind = + CodeCompleteConsumer::OverloadCandidate::CandidateKind::CK_Function; +}; +llvm::raw_ostream &operator<<(llvm::raw_ostream &, + const SignatureQualitySignals &); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp index 67f54ebfbb7..d59754a29dd 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp @@ -1515,6 +1515,28 @@ TEST(CompletionTest, CompletionTokenRange) { } } +TEST(SignatureHelpTest, OverloadsOrdering) { + const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, float y); + void foo(float x, int y); + void foo(float x, float y); + void foo(int x, int y = 0); + int main() { foo(^); } + )cpp"); + EXPECT_THAT( + Results.signatures, + ElementsAre( + Sig("foo(int x) -> void", {"int x"}), + Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}), + Sig("foo(float x, int y) -> void", {"float x", "int y"}), + Sig("foo(int x, float y) -> void", {"int x", "float y"}), + Sig("foo(float x, float y) -> void", {"float x", "float y"}))); + // We always prefer the first signature. + EXPECT_EQ(0, Results.activeSignature); + EXPECT_EQ(0, Results.activeParameter); +} + } // namespace } // namespace clangd } // namespace clang |
