tom-anders created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This implements the 1st heuristic mentioned in
https://github.com/clangd/clangd/issues/968#issuecomment-1002242704:
When completing a function that names a non-static member of a class, and we
are not inside that class's scope, assume the reference will not be a call (and
thus don't add the snippetSuffix)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137040
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -500,6 +500,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;
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -291,7 +291,8 @@
// computed from the first candidate, in the constructor.
// Others vary per candidate, so add() must be called for remaining candidates.
struct CodeCompletionBuilder {
- CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,
+ CodeCompletionBuilder(ASTContext *ASTCtx, DeclContext *SemaDeclCtx,
+ const CompletionCandidate &C,
CodeCompletionString *SemaCCS,
llvm::ArrayRef<std::string> QueryScopes,
const IncludeInserter &Includes,
@@ -299,13 +300,15 @@
CodeCompletionContext::Kind ContextKind,
const CodeCompleteOptions &Opts,
bool IsUsingDeclaration, tok::TokenKind NextTokenKind)
- : ASTCtx(ASTCtx),
+ : ASTCtx(ASTCtx), SemaDeclCtx(SemaDeclCtx),
EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
- IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
+ ContextKind(ContextKind), IsUsingDeclaration(IsUsingDeclaration),
+ NextTokenKind(NextTokenKind) {
Completion.Deprecated = true; // cleared by any non-deprecated overload.
add(C, SemaCCS);
if (C.SemaResult) {
assert(ASTCtx);
+ assert(SemaDeclCtx);
Completion.Origin |= SymbolOrigin::AST;
Completion.Name = std::string(llvm::StringRef(SemaCCS->getTypedText()));
Completion.FilterText = SemaCCS->getAllTypedText();
@@ -412,6 +415,8 @@
getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix,
&Completion.RequiredQualifier, IsPattern);
S.ReturnType = getReturnType(*SemaCCS);
+ maybeClearSnippetSuffixForMethodFunctionPointer(*C.SemaResult,
+ S.SnippetSuffix);
} else if (C.IndexResult) {
S.Signature = std::string(C.IndexResult->Signature);
S.SnippetSuffix = std::string(C.IndexResult->CompletionSnippetSuffix);
@@ -570,11 +575,57 @@
return "(â¦)";
}
+ // Clear snippet when completing a non-static member function (and not via
+ // dot/arrow member access) and we're not inside that class' scope
+ void clearSnippetSuffixWhenNotInsideClassScope(
+ const CodeCompletionResult &SemaResult, std::string &SnippetSuffix) {
+ if (SemaResult.Kind != CodeCompletionResult::RK_Declaration ||
+ // This check makes sure we only consider Foo::^ but not foo->^ here
+ ContextKind != CodeCompletionContext::Kind::CCC_Symbol)
+ return;
+
+ const auto *CompletedMethod =
+ llvm::dyn_cast<CXXMethodDecl>(SemaResult.getDeclaration());
+ if (!CompletedMethod || CompletedMethod->isStatic())
+ return;
+
+ const auto *CurrentClassScope = [&]() -> const CXXRecordDecl * {
+ for (DeclContext *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent()) {
+ const auto *CtxMethod = llvm::dyn_cast<CXXMethodDecl>(Ctx);
+ if (CtxMethod && !CtxMethod->getParent()->isLambda()) {
+ return CtxMethod->getParent();
+ }
+ }
+ return nullptr;
+ }();
+
+ bool InsideCompletedClassScope =
+ CurrentClassScope &&
+ (CurrentClassScope == CompletedMethod->getParent() ||
+ CurrentClassScope->isDerivedFrom(CompletedMethod->getParent()));
+ if (InsideCompletedClassScope)
+ return;
+
+ SnippetSuffix.clear();
+ }
+
+ // Some heuristics for when we don't want to add a snippet when completing
+ // a member function
+ void maybeClearSnippetSuffixForMethodFunctionPointer(
+ const CodeCompletionResult &SemaResult, std::string &SnippetSuffix) {
+ clearSnippetSuffixWhenNotInsideClassScope(SemaResult, SnippetSuffix);
+ // TODO Implement 2nd heuristic mentioned in
+ // https://github.com/clangd/clangd/issues/968#issuecomment-1002242704
+ }
+
// ASTCtx can be nullptr if not run with sema.
ASTContext *ASTCtx;
+ // SemaDeclCtx can be nullptr if not run with sema.
+ DeclContext *SemaDeclCtx;
CodeCompletion Completion;
llvm::SmallVector<BundledEntry, 1> Bundled;
bool EnableFunctionArgSnippets;
+ CodeCompletionContext::Kind ContextKind;
// No snippets will be generated for using declarations and when the function
// arguments are already present.
bool IsUsingDeclaration;
@@ -1934,7 +1985,8 @@
: nullptr;
if (!Builder)
Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
- Item, SemaCCS, QueryScopes, *Inserter, FileName,
+ Recorder ? Recorder->CCSema->CurContext : nullptr, Item,
+ SemaCCS, QueryScopes, *Inserter, FileName,
CCContextKind, Opts, IsUsingDeclaration, NextTokenKind);
else
Builder->add(Item, SemaCCS);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
