kadircet updated this revision to Diff 160097.
kadircet marked 3 inline comments as done.
kadircet added a comment.
Herald added a subscriber: mgrang.
- Resolve discussions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50555
Files:
clangd/CodeComplete.cpp
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1438,6 +1438,28 @@
}
}
+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
Index: clangd/Quality.h
===================================================================
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -163,6 +163,16 @@
/// 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
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -401,5 +401,17 @@
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
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -128,16 +128,16 @@
/// 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;
@@ -153,6 +153,8 @@
ParameterInformation Info;
Info.label = Chunk.Text;
Parameters.push_back(std::move(Info));
+ Signal.ContainsActiveParameter = true;
+ Signal.NumberOfOptionalParameters++;
break;
}
default:
@@ -679,6 +681,41 @@
llvm::unique_function<void()> ResultsCallback;
};
+using ScoredSignature =
+ std::pair<SignatureQualitySignals, SignatureInformation>;
+struct ScoredSignatureGreater {
+ bool operator()(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;
+ }
+};
+
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
@@ -692,7 +729,9 @@
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.
@@ -706,11 +745,16 @@
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(),
+ ScoredSignatureGreater());
+ for (const auto &SS : ScoredSignatures) {
+ SigHelp.signatures.push_back(SS.second);
+ }
}
GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
@@ -720,14 +764,15 @@
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) {
@@ -749,14 +794,16 @@
ParameterInformation Info;
Info.label = Chunk.Text;
Result.parameters.push_back(std::move(Info));
+ Signal.NumberOfParameters++;
+ Signal.ContainsActiveParameter = true;
break;
}
case CodeCompletionString::CK_Optional: {
// The rest of the parameters are defaulted/optional.
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:
@@ -770,7 +817,8 @@
Result.label += " -> ";
Result.label += ReturnType;
}
- return Result;
+ dlog("Signal for {0}: {1}", Result, Signal);
+ return {Signal, Result};
}
SignatureHelp &SigHelp;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
