zyounan created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added reviewers: nridge, kadircet.
zyounan published this revision for review.
zyounan added inline comments.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:130
+ return false;
+ return true;
+ }();
----------------
I was cringed that if we should refine the logic based on `CursorKind`: It is
from libclang; The meaning is sometimes kind of opaque (to me, I don't know it
very clearly TBH) like `CXCursor_NotImplemented`...
We have a workaround from D128621 <https://reviews.llvm.org/D128621> that makes
$0 no longer being
a placeholder to conform a vscode feature. However, we have to
refine the logic as it may suppress the last parameter placeholder
for constructor of base class because not all patterns of completion
are compound statements.
This fixes clangd/clangd#1479
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145319
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeCompletionStrings.cpp
clang-tools-extra/clangd/CodeCompletionStrings.h
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
@@ -3450,6 +3450,22 @@
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("while_foo"),
snippetSuffix("(${1:int a}, ${2:int b})"))));
+
+ Results = completions(R"cpp(
+ struct Base {
+ Base(int a, int b) {}
+ };
+
+ struct Derived : Base {
+ Derived() : Base^
+ };
+ )cpp",
+ /*IndexSymbols=*/{}, Options);
+ // Constructors from base classes are a kind of pattern that shouldn't end
+ // with $0.
+ EXPECT_THAT(Results.Completions,
+ Contains(AllOf(named("Base"),
+ snippetSuffix("(${1:int a}, ${2:int b})"))));
}
TEST(CompletionTest, WorksWithNullType) {
Index: clang-tools-extra/clangd/CodeCompletionStrings.h
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.h
+++ clang-tools-extra/clangd/CodeCompletionStrings.h
@@ -47,7 +47,8 @@
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
std::string *Snippet,
std::string *RequiredQualifiers = nullptr,
- bool CompletingPattern = false);
+ bool CompletingPattern = false,
+ const CodeCompletionResult *CCR = nullptr);
/// Assembles formatted documentation for a completion result. This includes
/// documentation comments and other relevant information like annotations.
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "CodeCompletionStrings.h"
+#include "clang-c/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
#include "clang/Basic/SourceManager.h"
@@ -96,7 +97,7 @@
void getSignature(const CodeCompletionString &CCS, std::string *Signature,
std::string *Snippet, std::string *RequiredQualifiers,
- bool CompletingPattern) {
+ bool CompletingPattern, const CodeCompletionResult *CCR) {
// 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();
@@ -111,6 +112,23 @@
return C.Kind == CodeCompletionString::CK_Placeholder;
});
}
+ // If the result kind of CCR is `RK_Pattern`, it doesn't always mean we're
+ // completing a chunk of statements. Constructors defined in base class, for
+ // example, are considered as a type of pattern, with the cursor type set to
+ // CXCursor_Constructor.
+ // We have to discriminate these cases manually in order to avoid providing
+ // incorrect placeholder `$0` which should have been a normal parameter.
+ bool ShouldPatchPlaceholder0 = CompletingPattern && [CCR] {
+ if (!CCR)
+ return true;
+ // The process of CCR construction employs `clang::getCursorKindForDecl` to
+ // obtain cursor kind for Decls, otherwise CursorKind would be set by
+ // constructor. Note that the default value is CXCursor_NotImplemented.
+ if (CCR->CursorKind == CXCursorKind::CXCursor_Constructor ||
+ CCR->CursorKind == CXCursorKind::CXCursor_Destructor)
+ return false;
+ return true;
+ }();
unsigned SnippetArg = 0;
bool HadObjCArguments = false;
bool HadInformativeChunks = false;
@@ -185,14 +203,14 @@
*Snippet += Chunk.Text;
break;
case CodeCompletionString::CK_Optional:
- assert(Chunk.Optional);
+ assert(Chunk.Optional);
// No need to create placeholders for default arguments in Snippet.
appendOptionalChunk(*Chunk.Optional, Signature);
break;
case CodeCompletionString::CK_Placeholder:
*Signature += Chunk.Text;
++SnippetArg;
- if (SnippetArg == CursorSnippetArg) {
+ if (SnippetArg == CursorSnippetArg && ShouldPatchPlaceholder0) {
// We'd like to make $0 a placeholder too, but vscode does not support
// this (https://github.com/microsoft/vscode/issues/152837).
*Snippet += "$0";
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -438,7 +438,7 @@
if (C.SemaResult) {
bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern;
getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix,
- &Completion.RequiredQualifier, IsPattern);
+ &Completion.RequiredQualifier, IsPattern, C.SemaResult);
if (!C.SemaResult->FunctionCanBeCall)
S.SnippetSuffix.clear();
S.ReturnType = getReturnType(*SemaCCS);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
