shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[clangd][CodeComplete] Improve FunctionCanBeCall

Browse Source 
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
This commit is contained in:
Younan Zhang 2023-08-06 19:47:41 +08:00
parent 1b8fb1a664
commit 23ef8bf9c0
8 changed files with 245 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clang-tools-extra/clangd/CodeComplete.cpp
View File

@ -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())

15
clang-tools-extra/clangd/CodeCompletionStrings.cpp
View File

@ -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>
@ -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();
@ -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.
@ -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:
@ -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,

5
clang-tools-extra/clangd/CodeCompletionStrings.h
View File

@ -42,6 +42,9 @@ 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
@ -49,7 +52,7 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D);
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

68
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
View File

@ -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}>()"))));
}
}

26
clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
View File

@ -25,11 +25,13 @@ public:
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);
}
@ -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");

138
clang/lib/Sema/SemaCodeComplete.cpp
View File

@ -310,6 +310,23 @@ public:
bool isInterestingDecl(const NamedDecl *ND,
bool &AsNestedNameSpecifier) const;
/// Decide whether or not a use of function Decl can be a call.
///
/// \param ND the function declaration.
///
/// \param BaseExprType the object type in a member access expression,
/// if any.
bool canFunctionBeCalled(const NamedDecl *ND, QualType BaseExprType) const;
/// Decide whether or not a use of member function Decl can be a call.
///
/// \param Method the function declaration.
///
/// \param BaseExprType the object type in a member access expression,
/// if any.
bool canCxxMethodBeCalled(const CXXMethodDecl *Method,
QualType BaseExprType) const;
/// Check whether the result is hidden by the Hiding declaration.
///
/// \returns true if the result is hidden and cannot be found, false if
@ -339,8 +356,11 @@ public:
///
/// \param InBaseClass whether the result was found in a base
/// class of the searched context.
///
/// \param BaseExprType the type of expression that precedes the "." or "->"
/// in a member access expression.
void AddResult(Result R, DeclContext *CurContext, NamedDecl *Hiding,
bool InBaseClass);
bool InBaseClass, QualType BaseExprType);
/// Add a new non-declaration result to this result set.
void AddResult(Result R);
@ -1262,8 +1282,69 @@ static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,
: OverloadCompare::Dominated;
}
bool ResultBuilder::canCxxMethodBeCalled(const CXXMethodDecl *Method,
QualType BaseExprType) const {
// Find the class scope that we're currently in.
// We could e.g. be inside a lambda, so walk up the DeclContext until we
// find a CXXMethodDecl.
DeclContext *CurContext = SemaRef.CurContext;
const auto *CurrentClassScope = [&]() -> const CXXRecordDecl * {
for (DeclContext *Ctx = CurContext; Ctx; Ctx = Ctx->getParent()) {
const auto *CtxMethod = llvm::dyn_cast<CXXMethodDecl>(Ctx);
if (CtxMethod && !CtxMethod->getParent()->isLambda()) {
return CtxMethod->getParent();
}
}
return nullptr;
}();
// If we're not inside the scope of the method's class, it can't be a call.
bool FunctionCanBeCall =
CurrentClassScope &&
(CurrentClassScope == Method->getParent() ||
CurrentClassScope->isDerivedFrom(Method->getParent()));
// We skip the following calculation for exceptions if it's already true.
if (FunctionCanBeCall)
return true;
// Exception: foo->FooBase::bar() or foo->Foo::bar() *is* a call.
if (const CXXRecordDecl *MaybeDerived =
BaseExprType.isNull() ? nullptr
: BaseExprType->getAsCXXRecordDecl()) {
auto *MaybeBase = Method->getParent();
FunctionCanBeCall =
MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase);
}
return FunctionCanBeCall;
}
bool ResultBuilder::canFunctionBeCalled(const NamedDecl *ND,
QualType BaseExprType) const {
// We apply heuristics only to CCC_Symbol:
// * CCC_{Arrow,Dot}MemberAccess reflect member access expressions:
// f.method() and f->method(). These are always calls.
// * A qualified name to a member function may *not* be a call. We have to
// subdivide the cases: For example, f.Base::method(), which is regarded as
// CCC_Symbol, should be a call.
// * Non-member functions and static member functions are always considered
// calls.
if (CompletionContext.getKind() == clang::CodeCompletionContext::CCC_Symbol) {
if (const auto *FuncTmpl = dyn_cast<FunctionTemplateDecl>(ND)) {
ND = FuncTmpl->getTemplatedDecl();
}
const auto *Method = dyn_cast<CXXMethodDecl>(ND);
if (Method && !Method->isStatic()) {
return canCxxMethodBeCalled(Method, BaseExprType);
}
}
return true;
}
void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
NamedDecl *Hiding, bool InBaseClass = false) {
NamedDecl *Hiding, bool InBaseClass = false,
QualType BaseExprType = QualType()) {
if (R.Kind != Result::RK_Declaration) {
// For non-declaration results, just add the result.
Results.push_back(R);
@ -1279,7 +1360,8 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
R.Availability == CXAvailability_Deprecated),
std::move(R.FixIts));
Result.ShadowDecl = Using;
AddResult(Result, CurContext, Hiding);
AddResult(Result, CurContext, Hiding, /*InBaseClass=*/false,
/*BaseExprType=*/BaseExprType);
return;
}
@ -1381,32 +1463,7 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
OverloadSet.Add(Method, Results.size());
}
// When completing a non-static member function (and not via
// dot/arrow member access) and we're not inside that class' scope,
// it can't be a call.
if (CompletionContext.getKind() == clang::CodeCompletionContext::CCC_Symbol) {
const auto *Method = dyn_cast<CXXMethodDecl>(R.getDeclaration());
if (Method && !Method->isStatic()) {
// Find the class scope that we're currently in.
// We could e.g. be inside a lambda, so walk up the DeclContext until we
// find a CXXMethodDecl.
const auto *CurrentClassScope = [&]() -> const CXXRecordDecl * {
for (DeclContext *Ctx = SemaRef.CurContext; Ctx;
Ctx = Ctx->getParent()) {
const auto *CtxMethod = llvm::dyn_cast<CXXMethodDecl>(Ctx);
if (CtxMethod && !CtxMethod->getParent()->isLambda()) {
return CtxMethod->getParent();
}
}
return nullptr;
}();
R.FunctionCanBeCall =
CurrentClassScope &&
(CurrentClassScope == Method->getParent() ||
CurrentClassScope->isDerivedFrom(Method->getParent()));
}
}
R.FunctionCanBeCall = canFunctionBeCalled(R.getDeclaration(), BaseExprType);
// Insert this result into the set of results.
Results.push_back(R);
@ -1680,7 +1737,7 @@ public:
bool InBaseClass) override {
ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
false, IsAccessible(ND, Ctx), FixIts);
Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass);
Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass, BaseType);
}
void EnteredContext(DeclContext *Ctx) override {
@ -3526,8 +3583,14 @@ CodeCompletionString *CodeCompletionResult::createCodeCompletionStringForDecl(
// Figure out which template parameters are deduced (or have default
// arguments).
llvm::SmallBitVector Deduced;
Sema::MarkDeducedTemplateParameters(Ctx, FunTmpl, Deduced);
// Note that we're creating a non-empty bit vector so that we can go
// through the loop below to omit default template parameters for non-call
// cases.
llvm::SmallBitVector Deduced(FunTmpl->getTemplateParameters()->size());
// Avoid running it if this is not a call: We should emit *all* template
// parameters.
if (FunctionCanBeCall)
Sema::MarkDeducedTemplateParameters(Ctx, FunTmpl, Deduced);
unsigned LastDeducibleArgument;
for (LastDeducibleArgument = Deduced.size(); LastDeducibleArgument > 0;
--LastDeducibleArgument) {
@ -3554,10 +3617,19 @@ CodeCompletionString *CodeCompletionResult::createCodeCompletionStringForDecl(
}
}
if (LastDeducibleArgument) {
if (LastDeducibleArgument || !FunctionCanBeCall) {
// Some of the function template arguments cannot be deduced from a
// function call, so we introduce an explicit template argument list
// containing all of the arguments up to the first deducible argument.
//
// Or, if this isn't a call, emit all the template arguments
// to disambiguate the (potential) overloads.
//
// FIXME: Detect cases where the function parameters can be deduced from
// the surrounding context, as per [temp.deduct.funcaddr].
// e.g.,
// template <class T> void foo(T);
// void (*f)(int) = foo;
Result.AddChunk(CodeCompletionString::CK_LeftAngle);
AddTemplateParameterChunks(Ctx, Policy, FunTmpl, Result,
LastDeducibleArgument);

11
clang/test/CodeCompletion/member-access.cpp
View File

@ -341,3 +341,14 @@ namespace members_using_fixits {
// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:339:10 %s -o - | FileCheck -check-prefix=CHECK-FIELD-DECLARED-VIA-USING %s
// CHECK-FIELD-DECLARED-VIA-USING: [#int#]field (requires fix-it: {339:8-339:9} to "->")
}
namespace function_can_be_call {
struct S {
template <typename T, typename U, typename V = int>
T foo(U, V);
};
&S::f
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:351:7 %s -o - | FileCheck -check-prefix=CHECK_FUNCTION_CAN_BE_CALL %s
// CHECK_FUNCTION_CAN_BE_CALL: COMPLETION: foo : [#T#]foo<<#typename T#>, <#typename U#>>(<#U#>, <#V#>)
}

31
clang/unittests/Sema/CodeCompleteTest.cpp
View File

@ -60,7 +60,10 @@ public:
for (unsigned I = 0; I < NumResults; ++I) {
auto R = Results[I];
if (R.Kind == CodeCompletionResult::RK_Declaration) {
if (const auto *FD = llvm::dyn_cast<FunctionDecl>(R.getDeclaration())) {
auto *ND = R.getDeclaration();
if (auto *Template = llvm::dyn_cast<FunctionTemplateDecl>(ND))
ND = Template->getTemplatedDecl();
if (const auto *FD = llvm::dyn_cast<FunctionDecl>(ND)) {
CompletedFunctionDecl D;
D.Name = FD->getNameAsString();
D.CanBeCall = R.FunctionCanBeCall;
@ -191,6 +194,10 @@ TEST(SemaCodeCompleteTest, FunctionCanBeCall) {
struct Foo {
static int staticMethod();
int method() const;
template <typename T, typename U, typename V = int>
T generic(U, V);
template <typename T, int U = 3>
static T staticGeneric();
Foo() {
this->$canBeCall^
$canBeCall^
@ -199,6 +206,8 @@ TEST(SemaCodeCompleteTest, FunctionCanBeCall) {
};
struct Derived : Foo {
using Foo::method;
using Foo::generic;
Derived() {
Foo::$canBeCall^
}
@ -207,15 +216,29 @@ TEST(SemaCodeCompleteTest, FunctionCanBeCall) {
struct OtherClass {
OtherClass() {
Foo f;
Derived d;
f.$canBeCall^
; // Prevent parsing as 'f.f'
f.Foo::$canBeCall^
&Foo::$cannotBeCall^
;
d.Foo::$canBeCall^
;
d.Derived::$canBeCall^
}
};
int main() {
Foo f;
Derived d;
f.$canBeCall^
; // Prevent parsing as 'f.f'
f.Foo::$canBeCall^
&Foo::$cannotBeCall^
;
d.Foo::$canBeCall^
;
d.Derived::$canBeCall^
}
)cpp");
@ -223,12 +246,16 @@ TEST(SemaCodeCompleteTest, FunctionCanBeCall) {
auto Results = CollectCompletedFunctions(Code.code(), P);
EXPECT_THAT(Results, Contains(AllOf(named("method"), isStatic(false),
canBeCall(true))));
EXPECT_THAT(Results, Contains(AllOf(named("generic"), isStatic(false),
canBeCall(true))));
}
for (const auto &P : Code.points("cannotBeCall")) {
auto Results = CollectCompletedFunctions(Code.code(), P);
EXPECT_THAT(Results, Contains(AllOf(named("method"), isStatic(false),
canBeCall(false))));
EXPECT_THAT(Results, Contains(AllOf(named("generic"), isStatic(false),
canBeCall(false))));
}
// static method can always be a call
@ -236,6 +263,8 @@ TEST(SemaCodeCompleteTest, FunctionCanBeCall) {
auto Results = CollectCompletedFunctions(Code.code(), P);
EXPECT_THAT(Results, Contains(AllOf(named("staticMethod"), isStatic(true),
canBeCall(true))));
EXPECT_THAT(Results, Contains(AllOf(named("staticGeneric"), isStatic(true),
canBeCall(true))));
}
}