diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -460,9 +460,9 @@ 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, + /*DropFunctionArguments=*/!C.SemaResult->FunctionCanBeCall, + /*RequiredQualifiers=*/&Completion.RequiredQualifier); S.ReturnType = getReturnType(*SemaCCS); if (C.SemaResult->Kind == CodeCompletionResult::RK_Declaration) if (const auto *D = C.SemaResult->getDeclaration()) diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -42,6 +42,10 @@ /// 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. /// +/// \p DropFunctionArguments indicates that the function call arguments should +/// be omitted from the \p Snippet. If enabled, 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 +53,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, CodeCompletionResult::ResultKind ResultKind, - CXCursorKind CursorKind, + CXCursorKind CursorKind, bool DropFunctionArguments = false, std::string *RequiredQualifiers = nullptr); /// Assembles formatted documentation for a completion result. This includes diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -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 #include @@ -118,7 +119,8 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, CodeCompletionResult::ResultKind ResultKind, - CXCursorKind CursorKind, std::string *RequiredQualifiers) { + CXCursorKind CursorKind, bool DropFunctionArguments, + 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::max(); @@ -138,6 +140,15 @@ unsigned SnippetArg = 0; bool HadObjCArguments = false; bool HadInformativeChunks = false; + + // This variable would be discarded directly at the end of this function. We + // store part of the chunks of snippets here if DropFunctionArguments is + // enabled. This way, the CCS iteration won't be interrupted, and the + // Signature for the function would be preserved. + // It is preferable if we don't produce the arguments at the clang site. But + // that would also lose the signatures, which could sometimes help users to + // understand the context. + std::string DiscardedSnippet; for (const auto &Chunk : CCS) { // Informative qualifier chunks only clutter completion results, skip // them. @@ -244,6 +255,10 @@ break; case CodeCompletionString::CK_LeftParen: case CodeCompletionString::CK_RightParen: + if (DropFunctionArguments && + ResultKind == CodeCompletionResult::RK_Declaration) + Snippet = &DiscardedSnippet; + LLVM_FALLTHROUGH; case CodeCompletionString::CK_LeftBracket: case CodeCompletionString::CK_RightBracket: case CodeCompletionString::CK_LeftBrace: diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -532,45 +532,67 @@ struct Foo { static int staticMethod(); int method() const; + template + void generic(T); + template + static T staticGeneric(); Foo() { - this->$keepSnippet^ - $keepSnippet^ - Foo::$keepSnippet^ + this->$canBeCall^ + $canBeCall^ + Foo::$canBeCall^ } }; struct Derived : Foo { 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^ } }; 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^ } )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("")))); + EXPECT_THAT( + Results.Completions, + Contains(AllOf(named("generic"), signature("(T)"), + snippetSuffix("<${1:typename T}, ${2:int 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("()")))); + EXPECT_THAT(Results.Completions, + Contains(AllOf( + named("generic"), signature("(T)"), + snippetSuffix("<${1:typename T}, ${2:int U}>(${3:T})")))); } // static method will always keep the snippet @@ -578,6 +600,10 @@ auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); EXPECT_THAT(Results.Completions, Contains(AllOf(named("staticMethod"), snippetSuffix("()")))); + EXPECT_THAT( + Results.Completions, + Contains(AllOf(named("staticGeneric"), signature("()"), + snippetSuffix("<${1:typename T}>()")))); } } diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -25,11 +25,13 @@ protected: void computeSignature(const CodeCompletionString &CCS, CodeCompletionResult::ResultKind ResultKind = - CodeCompletionResult::ResultKind::RK_Declaration) { + CodeCompletionResult::ResultKind::RK_Declaration, + bool DropFunctionArguments = false) { Signature.clear(); Snippet.clear(); getSignature(CCS, &Signature, &Snippet, ResultKind, /*CursorKind=*/CXCursorKind::CXCursor_NotImplemented, + /*DropFunctionArguments=*/DropFunctionArguments, /*RequiredQualifiers=*/nullptr); } @@ -156,6 +158,27 @@ 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, + /*DropFunctionArguments=*/true); + EXPECT_EQ(Signature, "(arg1, arg2)"); + EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>"); +} + TEST_F(CompletionStringTest, IgnoreInformativeQualifier) { Builder.AddTypedTextChunk("X"); Builder.AddInformativeChunk("info ok"); diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -338,8 +338,11 @@ /// /// \param InBaseClass whether the result was found in a base /// class of the searched context. + /// + /// \param BaseType 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 BaseType); /// Add a new non-declaration result to this result set. void AddResult(Result R); @@ -1262,7 +1265,8 @@ } void ResultBuilder::AddResult(Result R, DeclContext *CurContext, - NamedDecl *Hiding, bool InBaseClass = false) { + NamedDecl *Hiding, bool InBaseClass = false, + QualType BaseType = QualType()) { if (R.Kind != Result::RK_Declaration) { // For non-declaration results, just add the result. Results.push_back(R); @@ -1380,11 +1384,13 @@ 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. + // Decide whether or not a non-static member function can be a call. if (CompletionContext.getKind() == clang::CodeCompletionContext::CCC_Symbol) { - const auto *Method = dyn_cast(R.getDeclaration()); + const NamedDecl *ND = R.getDeclaration(); + if (const auto *FuncTmpl = dyn_cast(ND)) { + ND = FuncTmpl->getTemplatedDecl(); + } + const auto *Method = dyn_cast(ND); 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 @@ -1400,10 +1406,24 @@ return nullptr; }(); + // 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. R.FunctionCanBeCall = CurrentClassScope && (CurrentClassScope == Method->getParent() || CurrentClassScope->isDerivedFrom(Method->getParent())); + + // If the member access "." or "->" is followed by a qualified Id and the + // object type is derived from or the same as that of the Id, then + // the candidate functions should be perceived as calls. + if (const CXXRecordDecl *MaybeDerived = nullptr; + !BaseType.isNull() && + (MaybeDerived = BaseType->getAsCXXRecordDecl())) { + auto *MaybeBase = Method->getParent(); + R.FunctionCanBeCall = + MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase); + } } } @@ -1679,7 +1699,7 @@ 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 { @@ -3550,10 +3570,12 @@ } } - 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. Result.AddChunk(CodeCompletionString::CK_LeftAngle); AddTemplateParameterChunks(Ctx, Policy, FunTmpl, Result, LastDeducibleArgument); diff --git a/clang/test/CodeCompletion/member-access.cpp b/clang/test/CodeCompletion/member-access.cpp --- a/clang/test/CodeCompletion/member-access.cpp +++ b/clang/test/CodeCompletion/member-access.cpp @@ -341,3 +341,14 @@ // 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 + V foo(T, U); + }; + + &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 : [#V#]foo<<#typename T#>, <#typename U#>{#, <#typename V#>#}>(<#T#>, <#U#>) +} diff --git a/clang/unittests/Sema/CodeCompleteTest.cpp b/clang/unittests/Sema/CodeCompleteTest.cpp --- a/clang/unittests/Sema/CodeCompleteTest.cpp +++ b/clang/unittests/Sema/CodeCompleteTest.cpp @@ -60,7 +60,10 @@ for (unsigned I = 0; I < NumResults; ++I) { auto R = Results[I]; if (R.Kind == CodeCompletionResult::RK_Declaration) { - if (const auto *FD = llvm::dyn_cast(R.getDeclaration())) { + auto *ND = R.getDeclaration(); + if (auto *Template = llvm::dyn_cast(ND)) + ND = Template->getTemplatedDecl(); + if (const auto *FD = llvm::dyn_cast(ND)) { CompletedFunctionDecl D; D.Name = FD->getNameAsString(); D.CanBeCall = R.FunctionCanBeCall; @@ -191,6 +194,10 @@ struct Foo { static int staticMethod(); int method() const; + template + void generic(T); + template + static T staticGeneric(); Foo() { this->$canBeCall^ $canBeCall^ @@ -207,15 +214,25 @@ struct OtherClass { OtherClass() { Foo f; + Derived d; f.$canBeCall^ + ; // Prevent parsing as 'f.f' + f.Foo::$canBeCall^ &Foo::$cannotBeCall^ + ; + d.Foo::$canBeCall^ } }; int main() { Foo f; + Derived d; f.$canBeCall^ + ; // Prevent parsing as 'f.f' + f.Foo::$canBeCall^ &Foo::$cannotBeCall^ + ; + d.Foo::$canBeCall^ } )cpp"); @@ -223,12 +240,16 @@ 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 +257,8 @@ 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)))); } }