[clang-tools-extra] r339547 - [clangd] Introduce scoring mechanism for SignatureInformations.
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 13 01:40:05 PDT 2018
Author: kadircet
Date: Mon Aug 13 01:40:05 2018
New Revision: 339547
URL: http://llvm.org/viewvc/llvm-project?rev=339547&view=rev
Log:
[clangd] Introduce scoring mechanism for SignatureInformations.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: mgrang, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50555
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
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=339547&r1=339546&r2=339547&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Aug 13 01:40:05 2018
@@ -129,16 +129,16 @@ toCompletionItemKind(CodeCompletionResul
/// 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 CodeCompleti
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;
Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=339547&r1=339546&r2=339547&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Mon Aug 13 01:40:05 2018
@@ -401,5 +401,17 @@ std::string sortText(float Score, llvm::
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
Modified: clang-tools-extra/trunk/clangd/Quality.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=339547&r1=339546&r2=339547&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.h (original)
+++ clang-tools-extra/trunk/clangd/Quality.h Mon Aug 13 01:40:05 2018
@@ -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
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=339547&r1=339546&r2=339547&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon Aug 13 01:40:05 2018
@@ -1515,6 +1515,28 @@ TEST(CompletionTest, CompletionTokenRang
}
}
+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
More information about the cfe-commits
mailing list