Skip to content

Commit

Permalink
[clangd][CodeComplete] Improve FunctionCanBeCall
Browse files Browse the repository at this point in the history
From two aspects:

- For function templates, emit additional template argument
placeholders in the context where it can't be a call in order
to specify an instantiation explicitly.

- Consider expressions with base type specifier such as
'Derived().Base::foo^' a function call.

Reviewed By: nridge

Differential Revision: https://reviews.llvm.org/D156605
  • Loading branch information
zyn0217 committed Sep 28, 2023
1 parent 1b8fb1a commit 23ef8bf
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 55 deletions.
6 changes: 3 additions & 3 deletions clang-tools-extra/clangd/CodeComplete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,9 @@ struct CodeCompletionBuilder {
bool IsConcept = false;
if (C.SemaResult) {
getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind,
C.SemaResult->CursorKind, &Completion.RequiredQualifier);
if (!C.SemaResult->FunctionCanBeCall)
S.SnippetSuffix.clear();
C.SemaResult->CursorKind,
/*IncludeFunctionArguments=*/C.SemaResult->FunctionCanBeCall,
/*RequiredQualifiers=*/&Completion.RequiredQualifier);
S.ReturnType = getReturnType(*SemaCCS);
if (C.SemaResult->Kind == CodeCompletionResult::RK_Declaration)
if (const auto *D = C.SemaResult->getDeclaration())
Expand Down
15 changes: 14 additions & 1 deletion clang-tools-extra/clangd/CodeCompletionStrings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "clang/AST/RawCommentList.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/JSON.h"
#include <limits>
#include <utility>
Expand Down Expand Up @@ -118,7 +119,8 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) {
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
std::string *Snippet,
CodeCompletionResult::ResultKind ResultKind,
CXCursorKind CursorKind, std::string *RequiredQualifiers) {
CXCursorKind CursorKind, bool IncludeFunctionArguments,
std::string *RequiredQualifiers) {
// Placeholder with this index will be $0 to mark final cursor position.
// Usually we do not add $0, so the cursor is placed at end of completed text.
unsigned CursorSnippetArg = std::numeric_limits<unsigned>::max();
Expand All @@ -138,6 +140,8 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
unsigned SnippetArg = 0;
bool HadObjCArguments = false;
bool HadInformativeChunks = false;

std::optional<unsigned> TruncateSnippetAt;
for (const auto &Chunk : CCS) {
// Informative qualifier chunks only clutter completion results, skip
// them.
Expand Down Expand Up @@ -243,6 +247,13 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
"CompletionItems");
break;
case CodeCompletionString::CK_LeftParen:
// We're assuming that a LeftParen in a declaration starts a function
// call, and arguments following the parenthesis could be discarded if
// IncludeFunctionArguments is false.
if (!IncludeFunctionArguments &&
ResultKind == CodeCompletionResult::RK_Declaration)
TruncateSnippetAt.emplace(Snippet->size());
LLVM_FALLTHROUGH;
case CodeCompletionString::CK_RightParen:
case CodeCompletionString::CK_LeftBracket:
case CodeCompletionString::CK_RightBracket:
Expand All @@ -264,6 +275,8 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
break;
}
}
if (TruncateSnippetAt)
*Snippet = Snippet->substr(0, *TruncateSnippetAt);
}

std::string formatDocumentation(const CodeCompletionString &CCS,
Expand Down
5 changes: 4 additions & 1 deletion clang-tools-extra/clangd/CodeCompletionStrings.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D);
/// If set, RequiredQualifiers is the text that must be typed before the name.
/// e.g "Base::" when calling a base class member function that's hidden.
///
/// If \p IncludeFunctionArguments is disabled, the \p Snippet will only
/// contain function name and template arguments, if any.
///
/// When \p ResultKind is RK_Pattern, the last placeholder will be $0,
/// indicating the cursor should stay there.
/// Note that for certain \p CursorKind like \p CXCursor_Constructor, $0 won't
/// be emitted in order to avoid overlapping normal parameters.
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
std::string *Snippet,
CodeCompletionResult::ResultKind ResultKind,
CXCursorKind CursorKind,
CXCursorKind CursorKind, bool IncludeFunctionArguments = true,
std::string *RequiredQualifiers = nullptr);

/// Assembles formatted documentation for a completion result. This includes
Expand Down
68 changes: 53 additions & 15 deletions clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,54 +530,92 @@ TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) {

Annotations Code(R"cpp(
struct Foo {
static int staticMethod();
int method() const;
static int staticMethod(int);
int method(int) const;
template <typename T, typename U, typename V = int>
T generic(U, V);
template <typename T, int U>
static T staticGeneric();
Foo() {
this->$keepSnippet^
$keepSnippet^
Foo::$keepSnippet^
this->$canBeCall^
$canBeCall^
Foo::$canBeCall^
}
};
struct Derived : Foo {
using Foo::method;
using Foo::generic;
Derived() {
Foo::$keepSnippet^
Foo::$canBeCall^
}
};
struct OtherClass {
OtherClass() {
Foo f;
f.$keepSnippet^
&Foo::$noSnippet^
Derived d;
f.$canBeCall^
; // Prevent parsing as 'f.f'
f.Foo::$canBeCall^
&Foo::$canNotBeCall^
;
d.Foo::$canBeCall^
;
d.Derived::$canBeCall^
}
};
int main() {
Foo f;
f.$keepSnippet^
&Foo::$noSnippet^
Derived d;
f.$canBeCall^
; // Prevent parsing as 'f.f'
f.Foo::$canBeCall^
&Foo::$canNotBeCall^
;
d.Foo::$canBeCall^
;
d.Derived::$canBeCall^
}
)cpp");
auto TU = TestTU::withCode(Code.code());

for (const auto &P : Code.points("noSnippet")) {
for (const auto &P : Code.points("canNotBeCall")) {
auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts);
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("method"), snippetSuffix(""))));
Contains(AllOf(named("method"), signature("(int) const"),
snippetSuffix(""))));
// We don't have any arguments to deduce against if this isn't a call.
// Thus, we should emit these deducible template arguments explicitly.
EXPECT_THAT(
Results.Completions,
Contains(AllOf(named("generic"),
signature("<typename T, typename U>(U, V)"),
snippetSuffix("<${1:typename T}, ${2:typename U}>"))));
}

for (const auto &P : Code.points("keepSnippet")) {
for (const auto &P : Code.points("canBeCall")) {
auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts);
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("method"), snippetSuffix("()"))));
Contains(AllOf(named("method"), signature("(int) const"),
snippetSuffix("(${1:int})"))));
EXPECT_THAT(
Results.Completions,
Contains(AllOf(named("generic"), signature("<typename T>(U, V)"),
snippetSuffix("<${1:typename T}>(${2:U}, ${3:V})"))));
}

// static method will always keep the snippet
for (const auto &P : Code.points()) {
auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts);
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("staticMethod"), snippetSuffix("()"))));
Contains(AllOf(named("staticMethod"), signature("(int)"),
snippetSuffix("(${1:int})"))));
EXPECT_THAT(Results.Completions,
Contains(AllOf(
named("staticGeneric"), signature("<typename T, int U>()"),
snippetSuffix("<${1:typename T}, ${2:int U}>()"))));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ class CompletionStringTest : public ::testing::Test {
protected:
void computeSignature(const CodeCompletionString &CCS,
CodeCompletionResult::ResultKind ResultKind =
CodeCompletionResult::ResultKind::RK_Declaration) {
CodeCompletionResult::ResultKind::RK_Declaration,
bool IncludeFunctionArguments = true) {
Signature.clear();
Snippet.clear();
getSignature(CCS, &Signature, &Snippet, ResultKind,
/*CursorKind=*/CXCursorKind::CXCursor_NotImplemented,
/*IncludeFunctionArguments=*/IncludeFunctionArguments,
/*RequiredQualifiers=*/nullptr);
}

Expand Down Expand Up @@ -156,6 +158,28 @@ TEST_F(CompletionStringTest, SnippetsInPatterns) {
EXPECT_EQ(Snippet, " ${1:name} = $0;");
}

TEST_F(CompletionStringTest, DropFunctionArguments) {
Builder.AddTypedTextChunk("foo");
Builder.AddChunk(CodeCompletionString::CK_LeftAngle);
Builder.AddPlaceholderChunk("typename T");
Builder.AddChunk(CodeCompletionString::CK_Comma);
Builder.AddPlaceholderChunk("int U");
Builder.AddChunk(CodeCompletionString::CK_RightAngle);
Builder.AddChunk(CodeCompletionString::CK_LeftParen);
Builder.AddPlaceholderChunk("arg1");
Builder.AddChunk(CodeCompletionString::CK_Comma);
Builder.AddPlaceholderChunk("arg2");
Builder.AddChunk(CodeCompletionString::CK_RightParen);

computeSignature(
*Builder.TakeString(),
/*ResultKind=*/CodeCompletionResult::ResultKind::RK_Declaration,
/*IncludeFunctionArguments=*/false);
// Arguments dropped from snippet, kept in signature.
EXPECT_EQ(Signature, "<typename T, int U>(arg1, arg2)");
EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>");
}

TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {
Builder.AddTypedTextChunk("X");
Builder.AddInformativeChunk("info ok");
Expand Down

0 comments on commit 23ef8bf

Please sign in to comment.