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 @@ -411,6 +411,8 @@ bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern; getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, &Completion.RequiredQualifier, IsPattern); + if (!C.SemaResult->FunctionCanBeCall) + S.SnippetSuffix.clear(); S.ReturnType = getReturnType(*SemaCCS); } else if (C.IndexResult) { S.Signature = std::string(C.IndexResult->Signature); 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 @@ -505,6 +505,63 @@ snippetSuffix("(${1:int i}, ${2:const float f})"))); } +TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) { + clangd::CodeCompleteOptions Opts; + Opts.EnableSnippets = true; + + Annotations Code(R"cpp( + struct Foo { + static int staticMethod(); + int method() const; + Foo() { + this->$keepSnippet^ + $keepSnippet^ + Foo::$keepSnippet^ + } + }; + + struct Derived : Foo { + Derived() { + Foo::$keepSnippet^ + } + }; + + struct OtherClass { + OtherClass() { + Foo f; + f.$keepSnippet^ + &Foo::$noSnippet^ + } + }; + + int main() { + Foo f; + f.$keepSnippet^ + &Foo::$noSnippet^ + } + )cpp"); + auto TU = TestTU::withCode(Code.code()); + + for (const auto &P : Code.points("noSnippet")) { + auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("method"), snippetSuffix("")))); + } + + for (const auto &P : Code.points("keepSnippet")) { + auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts); + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("method"), snippetSuffix("()")))); + } + + // 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("()")))); + } +} + TEST(CompletionTest, NoSnippetsInUsings) { clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; diff --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h --- a/clang/include/clang/Sema/CodeCompleteConsumer.h +++ b/clang/include/clang/Sema/CodeCompleteConsumer.h @@ -850,6 +850,12 @@ /// rather than a use of that entity. bool DeclaringEntity : 1; + /// When completing a function, whether it can be a call. This will usually be + /// true, but we have some heuristics, e.g. when a pointer to a non-static + /// member function is completed outside of that class' scope, it can never + /// be a call. + bool FunctionCanBeCall : 1; + /// If the result should have a nested-name-specifier, this is it. /// When \c QualifierIsInformative, the nested-name-specifier is /// informative rather than required. @@ -876,7 +882,7 @@ FixIts(std::move(FixIts)), Hidden(false), InBaseClass(false), QualifierIsInformative(QualifierIsInformative), StartsNestedNameSpecifier(false), AllParametersAreInformative(false), - DeclaringEntity(false), Qualifier(Qualifier) { + DeclaringEntity(false), FunctionCanBeCall(true), Qualifier(Qualifier) { // FIXME: Add assert to check FixIts range requirements. computeCursorKindAndAvailability(Accessible); } @@ -886,7 +892,8 @@ : Keyword(Keyword), Priority(Priority), Kind(RK_Keyword), CursorKind(CXCursor_NotImplemented), Hidden(false), InBaseClass(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), - AllParametersAreInformative(false), DeclaringEntity(false) {} + AllParametersAreInformative(false), DeclaringEntity(false), + FunctionCanBeCall(true) {} /// Build a result that refers to a macro. CodeCompletionResult(const IdentifierInfo *Macro, @@ -896,7 +903,7 @@ CursorKind(CXCursor_MacroDefinition), Hidden(false), InBaseClass(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), AllParametersAreInformative(false), DeclaringEntity(false), - MacroDefInfo(MI) {} + FunctionCanBeCall(true), MacroDefInfo(MI) {} /// Build a result that refers to a pattern. CodeCompletionResult( @@ -908,7 +915,7 @@ CursorKind(CursorKind), Availability(Availability), Hidden(false), InBaseClass(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), AllParametersAreInformative(false), - DeclaringEntity(false) {} + DeclaringEntity(false), FunctionCanBeCall(true) {} /// Build a result that refers to a pattern with an associated /// declaration. @@ -917,7 +924,7 @@ : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern), Hidden(false), InBaseClass(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), AllParametersAreInformative(false), - DeclaringEntity(false) { + DeclaringEntity(false), FunctionCanBeCall(true) { computeCursorKindAndAvailability(); } 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 @@ -1379,6 +1379,33 @@ 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(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(Ctx); + if (CtxMethod && !CtxMethod->getParent()->isLambda()) { + return CtxMethod->getParent(); + } + } + return nullptr; + }(); + + R.FunctionCanBeCall = + CurrentClassScope && + (CurrentClassScope == Method->getParent() || + CurrentClassScope->isDerivedFrom(Method->getParent())); + } + } + // Insert this result into the set of results. Results.push_back(R); 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 @@ -23,6 +23,8 @@ using namespace clang; using namespace clang::tooling; +using ::testing::AllOf; +using ::testing::Contains; using ::testing::Each; using ::testing::UnorderedElementsAre; @@ -36,6 +38,51 @@ std::string PtrDiffType; }; +struct CompletedFunctionDecl { + std::string Name; + bool IsStatic; + bool CanBeCall; +}; +MATCHER_P(named, name, "") { return arg.Name == name; } +MATCHER_P(isStatic, value, "") { return arg.IsStatic == value; } +MATCHER_P(canBeCall, value, "") { return arg.CanBeCall == value; } + +class SaveCompletedFunctions : public CodeCompleteConsumer { +public: + SaveCompletedFunctions(std::vector &CompletedFuncDecls) + : CodeCompleteConsumer(/*CodeCompleteOpts=*/{}), + CompletedFuncDecls(CompletedFuncDecls), + CCTUInfo(std::make_shared()) {} + + void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context, + CodeCompletionResult *Results, + unsigned NumResults) override { + 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())) { + CompletedFunctionDecl D; + D.Name = FD->getNameAsString(); + D.CanBeCall = R.FunctionCanBeCall; + D.IsStatic = FD->isStatic(); + CompletedFuncDecls.emplace_back(std::move(D)); + } + } + } + } + +private: + CodeCompletionAllocator &getAllocator() override { + return CCTUInfo.getAllocator(); + } + + CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; } + + std::vector &CompletedFuncDecls; + + CodeCompletionTUInfo CCTUInfo; +}; + class VisitedContextFinder : public CodeCompleteConsumer { public: VisitedContextFinder(CompletionContext &ResultCtx) @@ -74,19 +121,19 @@ class CodeCompleteAction : public SyntaxOnlyAction { public: - CodeCompleteAction(ParsedSourceLocation P, CompletionContext &ResultCtx) - : CompletePosition(std::move(P)), ResultCtx(ResultCtx) {} + CodeCompleteAction(ParsedSourceLocation P, CodeCompleteConsumer *Consumer) + : CompletePosition(std::move(P)), Consumer(Consumer) {} bool BeginInvocation(CompilerInstance &CI) override { CI.getFrontendOpts().CodeCompletionAt = CompletePosition; - CI.setCodeCompletionConsumer(new VisitedContextFinder(ResultCtx)); + CI.setCodeCompletionConsumer(Consumer); return true; } private: // 1-based code complete position ; ParsedSourceLocation CompletePosition; - CompletionContext &ResultCtx; + CodeCompleteConsumer *Consumer; }; ParsedSourceLocation offsetToPosition(llvm::StringRef Code, size_t Offset) { @@ -103,7 +150,7 @@ CompletionContext ResultCtx; clang::tooling::runToolOnCodeWithArgs( std::make_unique(offsetToPosition(Code, Offset), - ResultCtx), + new VisitedContextFinder(ResultCtx)), Code, {"-std=c++11"}, TestCCName); return ResultCtx; } @@ -129,6 +176,69 @@ return Types; } +std::vector +CollectCompletedFunctions(StringRef Code, std::size_t Point) { + std::vector Result; + clang::tooling::runToolOnCodeWithArgs( + std::make_unique(offsetToPosition(Code, Point), + new SaveCompletedFunctions(Result)), + Code, {"-std=c++11"}, TestCCName); + return Result; +} + +TEST(SemaCodeCompleteTest, FunctionCanBeCall) { + llvm::Annotations Code(R"cpp( + struct Foo { + static int staticMethod(); + int method() const; + Foo() { + this->$canBeCall^ + $canBeCall^ + Foo::$canBeCall^ + } + }; + + struct Derived : Foo { + Derived() { + Foo::$canBeCall^ + } + }; + + struct OtherClass { + OtherClass() { + Foo f; + f.$canBeCall^ + &Foo::$cannotBeCall^ + } + }; + + int main() { + Foo f; + f.$canBeCall^ + &Foo::$cannotBeCall^ + } + )cpp"); + + for (const auto &P : Code.points("canBeCall")) { + auto Results = CollectCompletedFunctions(Code.code(), P); + EXPECT_THAT(Results, Contains(AllOf(named("method"), 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)))); + } + + // static method can always be a call + for (const auto &P : Code.points()) { + auto Results = CollectCompletedFunctions(Code.code(), P); + EXPECT_THAT(Results, Contains(AllOf(named("staticMethod"), isStatic(true), + canBeCall(true)))); + } +} + TEST(SemaCodeCompleteTest, VisitedNSForValidQualifiedId) { auto VisitedNS = runCodeCompleteOnCode(R"cpp( namespace ns1 {}